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

[Simon Glass]

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?

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