RE: [PATCH dev-5.0 v1 2/3] ARM: dts: nuvoton: Add Quanta GSJ BMC machine.

["Fran Hsu (徐誌謙)" <Fran.Hsu@quantatw.com>]

Hi Tomer,
	I think the main idea is to provide a full and common pinctrl file (just like nuvoton-npcm7xx-gpio.dtsi) for each NPCM7xx BMC family.
The device tree maintainers can reference the GPIO configuration from this file what they needed.
Once if the driver is changed, maybe the only change is the common pinctrl file.
But if there is no common pinctrl file, each device tree maintainers have to modify the gpio-dtsi file.
And as your opinion, there will be a lots of gpio-dtsi files for different boards or different vendors.
And most of gpio-dtsi files will include same GPIO setting.

Thanks,
Fran

>
>Hi Benjamin,
>
>On Sat, 30 Mar 2019 at 02:31, Benjamin Fair
<mailto:benjaminfair@google.com> wrote:
>Hi Fran,
>
>Thanks for sending these patches!
>
>On Thu, Mar 28, 2019 at 8:06 AM <mailto:fran.hsu@quantatw.com> wrote:
>>
>> From: FranHsu <mailto:Fran.Hsu@quantatw.com>
>>
>> Add pinctrl definition file for quanta-gsj BMC device tree.
>>
>> Signed-off-by: FranHsu <mailto:Fran.Hsu@quantatw.com>
>> ---
>>  .../boot/dts/nuvoton-npcm730-gsj-gpio.dtsi    | 2591
+++++++++++++++++
>>  1 file changed, 2591 insertions(+)
>>  create mode 100644 arch/arm/boot/dts/nuvoton-npcm730-gsj-gpio.dtsi
>
>Would it make more sense to merge this into the existing
>nuvoton-npcm750-gpio.dtsi file? That seems like it would make it easier to
>maintain the definitions and these are a superset of the existing ones, so it
>shouldn't conflict.
>
>the GPIO configuration depend on the server board that using it, we think it is
better that each
>server board or a family of server boards will have its own GPIO device tree
definition.
>
>We know that the GPIO device tree name and placement is confusing,
because is called nuvoton-npcm750-gpio.dtsi and not
nuvoton-npcm750-gpio-evb.dtsi
>and placed at nuvoton-npcm750.dtsi but should move the
nuvoton-npcm750-evb.dts file.
>
>we will modify the gpio npcm750 device tree soon.
>
>Fran,
>
>Does the nuvoton-npcm730-gsj-gpio.dtsi set GPIO configuration you are using
or will use in the future on Quanta server boards?
>For example I see you defined "gpio0o_pins" pin configuration, and you are
not using it in your dts file. will you use it in the future or in other Quanta
based NPCM7xx BMC boards?
>if not I think you should remove it. eqtully I think you should defined only the
GPIO/PIN configuration you are using, (I attend to do it at
nuvoton-npcm750-gpio.dtsi as well)
>
>
>>
>> diff --git a/arch/arm/boot/dts/nuvoton-npcm730-gsj-gpio.dtsi
b/arch/arm/boot/dts/nuvoton-npcm730-gsj-gpio.dtsi
>> new file mode 100644
>> index 000000000000..b66ea5099e5e
>> --- /dev/null
>> +++ b/arch/arm/boot/dts/nuvoton-npcm730-gsj-gpio.dtsi
>> @@ -0,0 +1,2591 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +// Copyright (c) 2018 Nuvoton Technology
mailto:tomer.maimon@nuvoton.com
>> +
>> +/ {
>> +       pinctrl: pinctrl@f0800000 {
>> +               gpio0o_pins: gpio0o-pins {
>> +                       pins = "GPIO0/IOX1DI";
>> +                       bias-disable;
>> +                       output-high;
>> +               };
>> +               gpio0ol_pins: gpio0ol-pins {
>> +                       pins = "GPIO0/IOX1DI";
>> +                       bias-disable;
>> +                       output-low;
>> +               };
>> +               gpio0od_pins: gpio0od-pins {
>> +                       pins = "GPIO0/IOX1DI";
>> +                       bias-disable;
>> +                       drive-open-drain;
>> +               };
>> +               gpio0pp_pins: gpio0pp-pins {
>> +                       pins = "GPIO0/IOX1DI";
>> +                       bias-disable;
>> +                       drive-push-pull;
>> +               };
>> +               gpio1_pins: gpio1-pins {
>> +                       pins = "GPIO1/IOX1LD";
>> +                       bias-disable;
>> +                       input-enable;
>> +               };
>> +               gpio1o_pins: gpio1o-pins {
>> +                       pins = "GPIO1/IOX1LD";
>> +                       bias-disable;
>> +                       output-high;
>> +               };
>> +               gpio1ol_pins: gpio1ol-pins {