[PATCH v3 2/3] timer: cadence: Add bind function to driver

[Sean Anderson]

On 10/14/21 2:24 PM, Simon Glass wrote:
> Hi Sean,
>
> On Thu, 14 Oct 2021 at 09:36, Sean Anderson <sean.anderson at seco.com> wrote:
>>
>>
>>
>> On 10/14/21 11:09 AM, Simon Glass wrote:
>> > Hi Michal,
>> >
>> > On Wed, 6 Oct 2021 at 08:19, Michal Simek <michal.simek at xilinx.com> wrote:
>> >>
>> >> When DT node has pwm-cells property it shouldn't be bind as timer driver
>> >> but as PWM driver. That's why make sure that this property is checked.
>> >>
>> >> Signed-off-by: Michal Simek <michal.simek at xilinx.com>
>> >> ---
>> >>
>> >> Changes in v3:
>> >> - New patch in series
>> >>
>> >>  drivers/timer/cadence-ttc.c | 12 ++++++++++++
>> >>  1 file changed, 12 insertions(+)
>> >>
>> >
>> > Why not have two compatible strings?
>>
>> As I understand it, because the hardware is the same, it should have the
>> same compatible string.
>>
>> >> diff --git a/drivers/timer/cadence-ttc.c b/drivers/timer/cadence-ttc.c
>> >> index 2f95d45ecd7a..2eff45060ad6 100644
>> >> --- a/drivers/timer/cadence-ttc.c
>> >> +++ b/drivers/timer/cadence-ttc.c
>> >> @@ -97,6 +97,17 @@ static int cadence_ttc_of_to_plat(struct udevice *dev)
>> >>         return 0;
>> >>  }
>> >>
>> >> +static int cadence_ttc_bind(struct udevice *dev)
>> >> +{
>> >> +       const char *cells;
>> >> +
>> >> +       cells = dev_read_prop(dev, "#pwm-cells", NULL);
>> >> +       if (cells)
>> >> +               return -ENODEV;
>> >
>> > We can read properties in bind() when necessary, but this is very
>> > strange...it seems like the bindings are messed up?
>>
>> This is the preferred way to select between a PWM and a timer driver.
>> See e.g. Rob's comment for xlnx,pwm at [1].
>
> I actually don't understand his comment. Is it the 'No...' one?

No, it's the

> If a PWM, perhaps you want a '#pwm-cells' property which can serve as
> the hint to configure as a PWM.

--Sean

>
> Anyway, it seems you understand what you are doing, and this is
> supposed to be supported by driver model.
>
> Reviewed-by: Simon Glass <sjg at chromium.org>
>
> Regards,
> Simon
>