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)  and Fan tach controller.
>> > 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>
>> > ---
>> >  .../devicetree/bindings/hwmon/npcm750-pwm-fan.txt  | 84 ++++++++++++++++++++++
>> >  1 file changed, 84 insertions(+)
>> >  create mode 100644 Documentation/devicetree/bindings/hwmon/npcm750-pwm-fan.txt
>> >
>> > diff --git a/Documentation/devicetree/bindings/hwmon/npcm750-pwm-fan.txt b/Documentation/devicetree/bindings/hwmon/npcm750-pwm-fan.txt
>> > new file mode 100644
>> > index 000000000000..a9eacda34f92
>> > --- /dev/null
>> > +++ b/Documentation/devicetree/bindings/hwmon/npcm750-pwm-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) controller modules,
>> > +Each PWM module has four PWM controller outputs, Totally 8 PWM controller outputs.
>> > +
>> > +The NPCM7xx has eight identical Fan tachometer controller modules,
>> > +Each Fan module has two Fan controller inputs, Totally 16 Fan controller inputs.
>>
>> Have you looked at other fan ctrlr bindings?f This looks like similar
>> h/w to ASpeed. Really, I'd like to see a common doc that describes 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 with 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
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  i will send toady),
and later on we will work on creating common binding document for the fan-nodes.

also I will like to know how to represent the fan-nodes properties in the common binding document,
the property name "aspeed,fan-tach-ch" is something related Aspeed.

Do I need to ask the Aspeed hwmon maintainer to modify his driver to use fan-tach-ch property? 
or to mention both fan-tach properties (NPCM and Aspeed) in the  common binding document ?

Thanks,

Tomer