Hi Rob,

On 26 June 2018 at 18:03, Rob Herring <robh@kernel.org> wrote:

On Mon, Jun 25, 2018 at 4:20 PM Tomer Maimon <tmaimon77@gmail.com> wrote:
>
> Hi Rob,
>
>
> On 25 June 2018 at 20:14, Rob Herring <robh@kernel.org> wrote:
>>
>> On Sun, Jun 24, 2018 at 03:41:54PM +0300, Tomer Maimon wrote:
>> > Added device tree binding documentation for Nuvoton BMC
>> > NPCM7xx Pulse Width Modulation (PWM)=C2=A0 and Fan tach contr= oller.
>> > The PWM controller can support upto 8 PWM output ports.
>> > The Fan tach controller can support upto 16 tachometer inputs= .
>> >
>> > Signed-off-by: Tomer Maimon <tmaimon77@gmail.com>
>> > ---
>> >=C2=A0 .../devicetree/bindings/hwmon/npcm750-pwm-fan.txt= =C2=A0 | 84 ++++++++++++++++++++++
>> >=C2=A0 1 file changed, 84 insertions(+)
>> >=C2=A0 create mode 100644 Documentation/devicetree/bindin= gs/hwmon/npcm750-pwm-fan.txt
>> >
>> > diff --git a/Documentation/devicetree/bindings/hwmon/npc= m750-pwm-fan.txt b/Documentation/devicetree/bindings/hwmon/npcm75= 0-pwm-fan.txt
>> > new file mode 100644
>> > index 000000000000..a9eacda34f92
>> > --- /dev/null
>> > +++ b/Documentation/devicetree/bindings/hwmon/npcm750-pw= m-fan.txt
>> > @@ -0,0 +1,84 @@
>> > +Nuvoton NPCM7xx PWM and Fan Tacho controller device driver >>
>> Bindings are for h/w, not drivers.
>>
>> > +
>> > +The NPCM7xx has two identical Pulse-width modulation (PWM) c= ontroller modules,
>> > +Each PWM module has four PWM controller outputs, Totally 8 P= WM controller outputs.
>> > +
>> > +The NPCM7xx has eight identical Fan tachometer controller mo= dules,
>> > +Each Fan module has two Fan controller inputs, Totally 16 Fa= n controller inputs.
>>
>> Have you looked at other fan ctrlr bindings?f This looks like simi= lar
>> h/w to ASpeed. Really, I'd like to see a common doc that descr= ibes the
>
>
> We do not have the same H/W as Aspeed, I believe in the near future we= will need to add
> more DT properties that will used only in the NPCM7xx module.

I didn't say it was the same. Both are multi-channel PWMs w= ith tach
inputs. Presumably, they can attach to the same types of fans as there
are only a limited number of types of fans and none of them are
specific to any fan controller.

>> structure and common properties.
>
>
> what do you mean by common structure and common properties?

When we have multiple bindings for the same class of device/hw, we define all the common parts in a common binding doc. This often
doesn't happen at first, so we end up with a variety of bindings until<= br> we see some commonality.

In this case, for structure, having sub-nodes for fans. As fans are
not specific to the controllers, their node should not be defined by
the controller binding. It's also how you describe the fan type,
number of fans, the PWM connections, the tach connections, etc.

Rob

Thanks a lot for the = clarification, indeed we can do a common binding doc.

is it O.K. = for now to stay with the same binding document (V5 - that=C2=A0 i will send= toady),

and later on we will work on creating=C2=A0common = binding document for the fan-nodes.

also I will like to know how to represent the=C2=A0fan-nodes properties= in the=C2=A0common binding document,

the property name "aspeed,fan-tach-ch" is something related Aspeed.

Do I need to ask the Aspeed hwmon main= tainer to modify his driver to use fan-tach-ch = property?= =C2=A0

or to mention both = fan-tach properties=C2=A0(NPCM = and Aspeed) in the=C2=A0

Thanks,

Tomer

=C2=A0