Re: [PATCH v3 2/3] timer: cadence: Add bind function to driver
On 10/14/21 2:24 PM, Simon Glass wrote:
Hi Sean,
On Thu, 14 Oct 2021 at 09:36, Sean Anderson wrote:
On 10/14/21 11:09 AM, Simon Glass wrote:
> Hi Michal,
>
> On Wed, 6 Oct 2021 at 08:19, Michal Simek 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
>> ---
>>
>> 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
Regards,
Simon
Re: [PATCH v3 2/3] timer: cadence: Add bind function to driver
Hi Sean,
On Thu, 14 Oct 2021 at 09:36, Sean Anderson wrote:
>
>
>
> On 10/14/21 11:09 AM, Simon Glass wrote:
> > Hi Michal,
> >
> > On Wed, 6 Oct 2021 at 08:19, Michal Simek 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
> >> ---
> >>
> >> 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
Regards,
Simon
Re: [PATCH v3 2/3] timer: cadence: Add bind function to driver
On 10/14/21 11:09 AM, Simon Glass wrote:
Hi Michal,
On Wed, 6 Oct 2021 at 08:19, Michal Simek 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
---
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].
--Sean
[1]
https://lore.kernel.org/linux-pwm/20210513021631.ga878...@robh.at.kernel.org/
+
+ return 0;
+}
+
static const struct timer_ops cadence_ttc_ops = {
.get_count = cadence_ttc_get_count,
};
@@ -114,4 +125,5 @@ U_BOOT_DRIVER(cadence_ttc) = {
.priv_auto = sizeof(struct cadence_ttc_priv),
.probe = cadence_ttc_probe,
.ops = _ttc_ops,
+ .bind = cadence_ttc_bind,
};
--
2.33.0
Regards,
Simon
Re: [PATCH v3 2/3] timer: cadence: Add bind function to driver
Hi Michal,
On Wed, 6 Oct 2021 at 08:19, Michal Simek 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
> ---
>
> Changes in v3:
> - New patch in series
>
> drivers/timer/cadence-ttc.c | 12
> 1 file changed, 12 insertions(+)
>
Why not have two compatible strings?
> 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?
> +
> + return 0;
> +}
> +
> static const struct timer_ops cadence_ttc_ops = {
> .get_count = cadence_ttc_get_count,
> };
> @@ -114,4 +125,5 @@ U_BOOT_DRIVER(cadence_ttc) = {
> .priv_auto = sizeof(struct cadence_ttc_priv),
> .probe = cadence_ttc_probe,
> .ops = _ttc_ops,
> + .bind = cadence_ttc_bind,
> };
> --
> 2.33.0
>
Regards,
Simon
Re: [PATCH v3 2/3] timer: cadence: Add bind function to driver
On 10/6/21 10:19 AM, Michal Simek 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
---
Changes in v3:
- New patch in series
drivers/timer/cadence-ttc.c | 12
1 file changed, 12 insertions(+)
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;
+
+ return 0;
+}
+
static const struct timer_ops cadence_ttc_ops = {
.get_count = cadence_ttc_get_count,
};
@@ -114,4 +125,5 @@ U_BOOT_DRIVER(cadence_ttc) = {
.priv_auto = sizeof(struct cadence_ttc_priv),
.probe = cadence_ttc_probe,
.ops = _ttc_ops,
+ .bind = cadence_ttc_bind,
};
Reviewed-by: Sean Anderson
[PATCH v3 2/3] timer: cadence: Add bind function to driver
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
---
Changes in v3:
- New patch in series
drivers/timer/cadence-ttc.c | 12
1 file changed, 12 insertions(+)
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;
+
+ return 0;
+}
+
static const struct timer_ops cadence_ttc_ops = {
.get_count = cadence_ttc_get_count,
};
@@ -114,4 +125,5 @@ U_BOOT_DRIVER(cadence_ttc) = {
.priv_auto = sizeof(struct cadence_ttc_priv),
.probe = cadence_ttc_probe,
.ops = _ttc_ops,
+ .bind = cadence_ttc_bind,
};
--
2.33.0
