mbox series

[v3,0/5] Basic pinctrl support for StarFive JH7110 RISC-V SoC

Message ID 20221220005529.34744-1-hal.feng@starfivetech.com
Headers
Series Basic pinctrl support for StarFive JH7110 RISC-V SoC |

Message

Hal Feng Dec. 20, 2022, 12:55 a.m. UTC 
  This patch series adds basic pinctrl support for StarFive JH7110 SoC.
You can simply get or review the patches at the link [1].

[1]: https://github.com/hal-feng/linux/commits/visionfive2-minimal

Changes since v2:
- Rebased on tag v6.1.
Patch 1:
- Renamed pinctrl-starfive-jh7110.h to
  starfive,jh7110-pinctrl.h. (by Krzysztof)
- Separated the register values in the binding header and stored them in
  a new file arch/riscv/boot/dts/starfive/jh7110-pinfunc.h. (by Krzysztof)
- Split patch 1 into sys part and aon part. Merged them into patch 2
  and patch 3 respectively.
Patch 2 & 3:
- Dropped "reg-names" and the description of "interrupts". Dropped quotes
  behind "$ref" and kept consisitent quotes. (by Krzysztof)
- Moved gpio properties behind interrupt properties.
- Moved "required" behind "patternProperties". (by Krzysztof)
- Rewrote the examples of bindings. (by Krzysztof and Emil)
- Added Co-developed-by tag for Emil.
- Dropped unused "clocks" property in patch 3.
Patch 4 & 5:
- Renamed "pinctrl-starfive.*" to "pinctrl-starfive-jh7110.*" and replaced
  all "starfive_" prefix with "jh7110_" in these files. (by Emil)
- Dropped macro GPIO_NUM_PER_WORD. (by Emil)
- Dropped unused flag member in starfive_pinctrl_soc_info structure. (by Emil)
- Renamed "pinctrl-jh7110-sys.c" to "pinctrl-starfive-jh7110-sys.c".
  Renamed "pinctrl-jh7110-aon.c" to "pinctrl-starfive-jh7110-aon.c". (by Emil)
- Added individual Kconfig options for sys and aon pinctrl drivers. (by Emil)
- Made the sys and aon pinctrl drivers be modules. (by Emil)
- Added "JH7110_" prefix for macro SYS_GPO_PDA_0_74_CFG,
  SYS_GPO_PDA_89_94_CFG and AON_GPO_PDA_0_5_CFG. (by Emil)
- Dropped jh7110_sys_pinctrl_probe() and jh7110_aon_pinctrl_probe().
  Got the match data in the common jh7110_pinctrl_probe() and used it
  to probe. (by Emil)
- Dropped the of_match_ptr macro(). (by Emil)
- Set the MODULE_LICENSE as "GPL" according to commit bf7fbeeae6db.

  v2: https://lore.kernel.org/all/20221118011108.70715-1-hal.feng@starfivetech.com/

Changes since v1:
- Rebased on tag v6.1-rc5.
- Dropped patch 22 and 23 since they were merged in v6.1-rc1.
- Removed some unused macros and register values which do not belong to
  bindings. Simplified pinctrl definitions in patch 24. (by Krzysztof)
- Split the bindings into sys pinctrl bindings and aon pinctrl bindings,
  and split patch 25 into two patches.
- Made the bindings follow generic pinctrl bindings. (by Krzysztof)
- Fixed some wrong indentation in bindings, and checked it with
  `make dt_binding_check`.
- Split the patch 26 into two patches which added sys and aon pinctrl
  driver respectively.
- Restructured the pinctrl drivers so made them follow generic pinctrl
  bindings. Rewrote `dt_node_to_map` and extracted the public code to make
  it clearer.

  v1: https://lore.kernel.org/all/20220929143225.17907-1-hal.feng@linux.starfivetech.com/

Jianlong Huang (5):
  riscv: dts: starfive: Add StarFive JH7110 pin function definitions
  dt-bindings: pinctrl: Add StarFive JH7110 sys pinctrl
  dt-bindings: pinctrl: Add StarFive JH7110 aon pinctrl
  pinctrl: starfive: Add StarFive JH7110 sys controller driver
  pinctrl: starfive: Add StarFive JH7110 aon controller driver

 .../pinctrl/starfive,jh7110-aon-pinctrl.yaml  | 126 +++
 .../pinctrl/starfive,jh7110-sys-pinctrl.yaml  | 142 +++
 MAINTAINERS                                   |   9 +-
 arch/riscv/boot/dts/starfive/jh7110-pinfunc.h | 308 ++++++
 drivers/pinctrl/starfive/Kconfig              |  33 +
 drivers/pinctrl/starfive/Makefile             |   4 +
 .../starfive/pinctrl-starfive-jh7110-aon.c    | 177 ++++
 .../starfive/pinctrl-starfive-jh7110-sys.c    | 449 ++++++++
 .../starfive/pinctrl-starfive-jh7110.c        | 979 ++++++++++++++++++
 .../starfive/pinctrl-starfive-jh7110.h        |  70 ++
 .../pinctrl/starfive,jh7110-pinctrl.h         | 137 +++
 11 files changed, 2431 insertions(+), 3 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/pinctrl/starfive,jh7110-aon-pinctrl.yaml
 create mode 100644 Documentation/devicetree/bindings/pinctrl/starfive,jh7110-sys-pinctrl.yaml
 create mode 100644 arch/riscv/boot/dts/starfive/jh7110-pinfunc.h
 create mode 100644 drivers/pinctrl/starfive/pinctrl-starfive-jh7110-aon.c
 create mode 100644 drivers/pinctrl/starfive/pinctrl-starfive-jh7110-sys.c
 create mode 100644 drivers/pinctrl/starfive/pinctrl-starfive-jh7110.c
 create mode 100644 drivers/pinctrl/starfive/pinctrl-starfive-jh7110.h
 create mode 100644 include/dt-bindings/pinctrl/starfive,jh7110-pinctrl.h


base-commit: 830b3c68c1fb1e9176028d02ef86f3cf76aa2476

Comments

Conor Dooley Jan. 12, 2023, 7:30 p.m. UTC | #1 
Hey Hal Feng,

On Tue, Dec 20, 2022 at 08:55:24AM +0800, Hal Feng wrote:
> This patch series adds basic pinctrl support for StarFive JH7110 SoC.
> You can simply get or review the patches at the link [1].

> [1]: https://github.com/hal-feng/linux/commits/visionfive2-minimal

Do you intend submitting a new version of the patchset to address the
comments about the bindings, or are you waiting for comments on the
code?

Thanks,
Conor.

> Changes since v2:
> - Rebased on tag v6.1.
> Patch 1:
> - Renamed pinctrl-starfive-jh7110.h to
>   starfive,jh7110-pinctrl.h. (by Krzysztof)
> - Separated the register values in the binding header and stored them in
>   a new file arch/riscv/boot/dts/starfive/jh7110-pinfunc.h. (by Krzysztof)
> - Split patch 1 into sys part and aon part. Merged them into patch 2
>   and patch 3 respectively.
> Patch 2 & 3:
> - Dropped "reg-names" and the description of "interrupts". Dropped quotes
>   behind "$ref" and kept consisitent quotes. (by Krzysztof)
> - Moved gpio properties behind interrupt properties.
> - Moved "required" behind "patternProperties". (by Krzysztof)
> - Rewrote the examples of bindings. (by Krzysztof and Emil)
> - Added Co-developed-by tag for Emil.
> - Dropped unused "clocks" property in patch 3.
> Patch 4 & 5:
> - Renamed "pinctrl-starfive.*" to "pinctrl-starfive-jh7110.*" and replaced
>   all "starfive_" prefix with "jh7110_" in these files. (by Emil)
> - Dropped macro GPIO_NUM_PER_WORD. (by Emil)
> - Dropped unused flag member in starfive_pinctrl_soc_info structure. (by Emil)
> - Renamed "pinctrl-jh7110-sys.c" to "pinctrl-starfive-jh7110-sys.c".
>   Renamed "pinctrl-jh7110-aon.c" to "pinctrl-starfive-jh7110-aon.c". (by Emil)
> - Added individual Kconfig options for sys and aon pinctrl drivers. (by Emil)
> - Made the sys and aon pinctrl drivers be modules. (by Emil)
> - Added "JH7110_" prefix for macro SYS_GPO_PDA_0_74_CFG,
>   SYS_GPO_PDA_89_94_CFG and AON_GPO_PDA_0_5_CFG. (by Emil)
> - Dropped jh7110_sys_pinctrl_probe() and jh7110_aon_pinctrl_probe().
>   Got the match data in the common jh7110_pinctrl_probe() and used it
>   to probe. (by Emil)
> - Dropped the of_match_ptr macro(). (by Emil)
> - Set the MODULE_LICENSE as "GPL" according to commit bf7fbeeae6db.
> 
>   v2: https://lore.kernel.org/all/20221118011108.70715-1-hal.feng@starfivetech.com/
> 
> Changes since v1:
> - Rebased on tag v6.1-rc5.
> - Dropped patch 22 and 23 since they were merged in v6.1-rc1.
> - Removed some unused macros and register values which do not belong to
>   bindings. Simplified pinctrl definitions in patch 24. (by Krzysztof)
> - Split the bindings into sys pinctrl bindings and aon pinctrl bindings,
>   and split patch 25 into two patches.
> - Made the bindings follow generic pinctrl bindings. (by Krzysztof)
> - Fixed some wrong indentation in bindings, and checked it with
>   `make dt_binding_check`.
> - Split the patch 26 into two patches which added sys and aon pinctrl
>   driver respectively.
> - Restructured the pinctrl drivers so made them follow generic pinctrl
>   bindings. Rewrote `dt_node_to_map` and extracted the public code to make
>   it clearer.
> 
>   v1: https://lore.kernel.org/all/20220929143225.17907-1-hal.feng@linux.starfivetech.com/
> 
> Jianlong Huang (5):
>   riscv: dts: starfive: Add StarFive JH7110 pin function definitions
>   dt-bindings: pinctrl: Add StarFive JH7110 sys pinctrl
>   dt-bindings: pinctrl: Add StarFive JH7110 aon pinctrl
>   pinctrl: starfive: Add StarFive JH7110 sys controller driver
>   pinctrl: starfive: Add StarFive JH7110 aon controller driver
> 
>  .../pinctrl/starfive,jh7110-aon-pinctrl.yaml  | 126 +++
>  .../pinctrl/starfive,jh7110-sys-pinctrl.yaml  | 142 +++
>  MAINTAINERS                                   |   9 +-
>  arch/riscv/boot/dts/starfive/jh7110-pinfunc.h | 308 ++++++
>  drivers/pinctrl/starfive/Kconfig              |  33 +
>  drivers/pinctrl/starfive/Makefile             |   4 +
>  .../starfive/pinctrl-starfive-jh7110-aon.c    | 177 ++++
>  .../starfive/pinctrl-starfive-jh7110-sys.c    | 449 ++++++++
>  .../starfive/pinctrl-starfive-jh7110.c        | 979 ++++++++++++++++++
>  .../starfive/pinctrl-starfive-jh7110.h        |  70 ++
>  .../pinctrl/starfive,jh7110-pinctrl.h         | 137 +++
>  11 files changed, 2431 insertions(+), 3 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/pinctrl/starfive,jh7110-aon-pinctrl.yaml
>  create mode 100644 Documentation/devicetree/bindings/pinctrl/starfive,jh7110-sys-pinctrl.yaml
>  create mode 100644 arch/riscv/boot/dts/starfive/jh7110-pinfunc.h
>  create mode 100644 drivers/pinctrl/starfive/pinctrl-starfive-jh7110-aon.c
>  create mode 100644 drivers/pinctrl/starfive/pinctrl-starfive-jh7110-sys.c
>  create mode 100644 drivers/pinctrl/starfive/pinctrl-starfive-jh7110.c
>  create mode 100644 drivers/pinctrl/starfive/pinctrl-starfive-jh7110.h
>  create mode 100644 include/dt-bindings/pinctrl/starfive,jh7110-pinctrl.h
> 
> 
> base-commit: 830b3c68c1fb1e9176028d02ef86f3cf76aa2476
> -- 
> 2.38.1
>
Hal Feng Jan. 17, 2023, 2:38 a.m. UTC | #2 
On Thu, 12 Jan 2023 19:30:05 +0000, Conor Dooley wrote:
> Hey Hal Feng,
> 
> On Tue, Dec 20, 2022 at 08:55:24AM +0800, Hal Feng wrote:
> > This patch series adds basic pinctrl support for StarFive JH7110 SoC.
> > You can simply get or review the patches at the link [1].
> 
> > [1]: https://github.com/hal-feng/linux/commits/visionfive2-minimal
> 
> Do you intend submitting a new version of the patchset to address the
> comments about the bindings, or are you waiting for comments on the
> code?

Sorry for the late reply. If no other comments, I will fix the current
known issues and submit a new version. Actually, I am busy with some
other things recently, and I will reply the comments as soon as possible.
Thanks.

Best regards,
Hal
Conor Dooley Jan. 17, 2023, 6:29 a.m. UTC | #3 
On 17 January 2023 02:38:11 GMT, Hal Feng <hal.feng@starfivetech.com> wrote:
>On Thu, 12 Jan 2023 19:30:05 +0000, Conor Dooley wrote:
>> Hey Hal Feng,
>> 
>> On Tue, Dec 20, 2022 at 08:55:24AM +0800, Hal Feng wrote:
>> > This patch series adds basic pinctrl support for StarFive JH7110 SoC.
>> > You can simply get or review the patches at the link [1].
>> 
>> > [1]: https://github.com/hal-feng/linux/commits/visionfive2-minimal
>> 
>> Do you intend submitting a new version of the patchset to address the
>> comments about the bindings, or are you waiting for comments on the
>> code?
>
>Sorry for the late reply. If no other comments, I will fix the current
>known issues and submit a new version. Actually, I am busy with some
>other things recently, and I will reply the comments as soon as possible.

no worries.
Just FYI, it'd be good to choose something like v6.2-rc1 as a base.
Looks like some header changes have landed
that make the current version not compile 

Thanks,
Conor.
Hal Feng Jan. 31, 2023, 12:59 a.m. UTC | #4 
On Tue, 20 Dec 2022 08:55:24 +0800, Hal Feng wrote:
> This patch series adds basic pinctrl support for StarFive JH7110 SoC.
> You can simply get or review the patches at the link [1].
> 
> [1]: https://github.com/hal-feng/linux/commits/visionfive2-minimal
> 

Hi, Linus,

Could you please help to review and give me some suggestions
for this patch series? Thank you for your time.

Best regards,
Hal
Linus Walleij Jan. 31, 2023, 1:13 p.m. UTC | #5 
On Tue, Dec 20, 2022 at 1:55 AM Hal Feng <hal.feng@starfivetech.com> wrote:

> This patch series adds basic pinctrl support for StarFive JH7110 SoC.
> You can simply get or review the patches at the link [1].
>
> [1]: https://github.com/hal-feng/linux/commits/visionfive2-minimal
>
> Changes since v2:
> - Rebased on tag v6.1.

Overall this looks OK, the DT bindings does not have any review from
the DT people but I think they had enough time to do that and were
properly CC:ed so not your fault.

However when I try to apply this to the pinctrl tree it fails,
for example it seems to depend on an entry in MAINTAINERS
which isn't upstream.

Can you please rebase the patches that are supposed to be
applied to the pinctrl tree (for example normally not patch 1, the DTS
patch) on my "devel" branch:
https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl.git/log/?h=devel

If you resend this quickly I can apply it pronto.

Yours,
Linus Walleij
Conor Dooley Jan. 31, 2023, 1:25 p.m. UTC | #6 
On Tue, Jan 31, 2023 at 02:13:44PM +0100, Linus Walleij wrote:
> On Tue, Dec 20, 2022 at 1:55 AM Hal Feng <hal.feng@starfivetech.com> wrote:
> 
> > This patch series adds basic pinctrl support for StarFive JH7110 SoC.
> > You can simply get or review the patches at the link [1].
> >
> > [1]: https://github.com/hal-feng/linux/commits/visionfive2-minimal
> >
> > Changes since v2:
> > - Rebased on tag v6.1.
> 
> Overall this looks OK, the DT bindings does not have any review from
> the DT people but I think they had enough time to do that and were
> properly CC:ed so not your fault.

FWIW, Rob left comments on patches 2 & 3 of the series:
https://lore.kernel.org/linux-riscv/20221220201920.GA1012864-robh@kernel.org/
https://lore.kernel.org/linux-riscv/20221220202003.GA1018798-robh@kernel.org

Relatively minor, but not ignored by the DT people.

> However when I try to apply this to the pinctrl tree it fails,
> for example it seems to depend on an entry in MAINTAINERS
> which isn't upstream.
> 
> Can you please rebase the patches that are supposed to be
> applied to the pinctrl tree (for example normally not patch 1, the DTS
> patch) on my "devel" branch:

Yah, the DTS patch needs to be dropped, the file it applies to doesn't
exist in any branch yet.

> https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl.git/log/?h=devel
> 
> If you resend this quickly I can apply it pronto.

It'd be nice to get this stuff into v6.3 if there is still time so that
there is one thing fewer blocking the base platform support!

Thanks Linus,
Conor.
Hal Feng Feb. 3, 2023, 7:33 a.m. UTC | #7 
On Tue, 31 Jan 2023 14:13:44 +0100, Linus Walleij wrote:
> On Tue, Dec 20, 2022 at 1:55 AM Hal Feng <hal.feng@starfivetech.com> wrote:
> 
>> This patch series adds basic pinctrl support for StarFive JH7110 SoC.
>> You can simply get or review the patches at the link [1].
>>
>> [1]: https://github.com/hal-feng/linux/commits/visionfive2-minimal
>>
>> Changes since v2:
>> - Rebased on tag v6.1.
> 
> Overall this looks OK, the DT bindings does not have any review from
> the DT people but I think they had enough time to do that and were
> properly CC:ed so not your fault.
> 
> However when I try to apply this to the pinctrl tree it fails,
> for example it seems to depend on an entry in MAINTAINERS
> which isn't upstream.
> 
> Can you please rebase the patches that are supposed to be
> applied to the pinctrl tree (for example normally not patch 1, the DTS
> patch) on my "devel" branch:
> https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl.git/log/?h=devel

I will drop the patch 1 and rebase the patches on your "devel" branch.
I will resend patch 1 after the device tree patches about StarFive JH7110 [1]
are merged.

[1] https://lore.kernel.org/all/20221220011247.35560-1-hal.feng@starfivetech.com/

Thank you for spending time to review this series.

Best regards,
Hal

> 
> If you resend this quickly I can apply it pronto.