[1/3] dt-bindings: pwm: kona: Add new compatible for new version pwm-kona

Message ID 1547184076-20521-2-git-send-email-sheetal.tigadoli@broadcom.com
State Changes Requested
Headers show
Series
  • Add support for PWM Configure and stablize for PWM kona
Related show

Commit Message

Sheetal Tigadoli Jan. 11, 2019, 5:21 a.m.
From: Praveen Kumar B <praveen.b@broadcom.com>

Add new compatible string for new version of pwm-kona

Signed-off-by: Praveen Kumar B <praveen.b@broadcom.com>
Reviewed-by: Ray Jui <ray.jui@broadcom.com>
Reviewed-by: Scott Branden <scott.branden@broadcom.com>
Signed-off-by: Sheetal Tigadoli <sheetal.tigadoli@broadcom.com>
---
 Documentation/devicetree/bindings/pwm/brcm,kona-pwm.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Uwe Kleine-König Jan. 11, 2019, 8:48 p.m. | #1
On Fri, Jan 11, 2019 at 10:51:14AM +0530, Sheetal Tigadoli wrote:
> From: Praveen Kumar B <praveen.b@broadcom.com>
> 
> Add new compatible string for new version of pwm-kona
> 
> Signed-off-by: Praveen Kumar B <praveen.b@broadcom.com>
> Reviewed-by: Ray Jui <ray.jui@broadcom.com>
> Reviewed-by: Scott Branden <scott.branden@broadcom.com>
> Signed-off-by: Sheetal Tigadoli <sheetal.tigadoli@broadcom.com>
> ---
>  Documentation/devicetree/bindings/pwm/brcm,kona-pwm.txt | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/pwm/brcm,kona-pwm.txt b/Documentation/devicetree/bindings/pwm/brcm,kona-pwm.txt
> index 8eae9fe..d37f697 100644
> --- a/Documentation/devicetree/bindings/pwm/brcm,kona-pwm.txt
> +++ b/Documentation/devicetree/bindings/pwm/brcm,kona-pwm.txt
> @@ -3,7 +3,7 @@ Broadcom Kona PWM controller device tree bindings
>  This controller has 6 channels.
>  
>  Required Properties :
> -- compatible: should contain "brcm,kona-pwm"
> +- compatible: should contain "brcm,kona-pwm" or "brcm,kona-pwm-v2"

Is v2 used on a newer generation of kona SoCs? On i.MX these variants
are usually named after the first SoC that came with the new variant. Is
this sensible here, too?

Best regards
Uwe
Scott Branden Jan. 11, 2019, 9:28 p.m. | #2
Hi Uwe

On 2019-01-11 12:48 p.m., Uwe Kleine-König wrote:
> On Fri, Jan 11, 2019 at 10:51:14AM +0530, Sheetal Tigadoli wrote:
>> From: Praveen Kumar B <praveen.b@broadcom.com>
>>
>> Add new compatible string for new version of pwm-kona
>>
>> Signed-off-by: Praveen Kumar B <praveen.b@broadcom.com>
>> Reviewed-by: Ray Jui <ray.jui@broadcom.com>
>> Reviewed-by: Scott Branden <scott.branden@broadcom.com>
>> Signed-off-by: Sheetal Tigadoli <sheetal.tigadoli@broadcom.com>
>> ---
>>   Documentation/devicetree/bindings/pwm/brcm,kona-pwm.txt | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/pwm/brcm,kona-pwm.txt b/Documentation/devicetree/bindings/pwm/brcm,kona-pwm.txt
>> index 8eae9fe..d37f697 100644
>> --- a/Documentation/devicetree/bindings/pwm/brcm,kona-pwm.txt
>> +++ b/Documentation/devicetree/bindings/pwm/brcm,kona-pwm.txt
>> @@ -3,7 +3,7 @@ Broadcom Kona PWM controller device tree bindings
>>   This controller has 6 channels.
>>   
>>   Required Properties :
>> -- compatible: should contain "brcm,kona-pwm"
>> +- compatible: should contain "brcm,kona-pwm" or "brcm,kona-pwm-v2"
> Is v2 used on a newer generation of kona SoCs? On i.MX these variants
> are usually named after the first SoC that came with the new variant. Is
> this sensible here, too?

It doesn't make as much sense here as different revs of the IP block are 
picked up based on various decisions.

A new SoC could decide to use an old version.

>
> Best regards
> Uwe
>
Uwe Kleine-König Jan. 12, 2019, 3:05 p.m. | #3
Hello Scott,

On Fri, Jan 11, 2019 at 01:28:45PM -0800, Scott Branden wrote:
> On 2019-01-11 12:48 p.m., Uwe Kleine-König wrote:
> > On Fri, Jan 11, 2019 at 10:51:14AM +0530, Sheetal Tigadoli wrote:
> > > From: Praveen Kumar B <praveen.b@broadcom.com>
> > > 
> > > Add new compatible string for new version of pwm-kona
> > > 
> > > Signed-off-by: Praveen Kumar B <praveen.b@broadcom.com>
> > > Reviewed-by: Ray Jui <ray.jui@broadcom.com>
> > > Reviewed-by: Scott Branden <scott.branden@broadcom.com>
> > > Signed-off-by: Sheetal Tigadoli <sheetal.tigadoli@broadcom.com>
> > > ---
> > >   Documentation/devicetree/bindings/pwm/brcm,kona-pwm.txt | 2 +-
> > >   1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/pwm/brcm,kona-pwm.txt b/Documentation/devicetree/bindings/pwm/brcm,kona-pwm.txt
> > > index 8eae9fe..d37f697 100644
> > > --- a/Documentation/devicetree/bindings/pwm/brcm,kona-pwm.txt
> > > +++ b/Documentation/devicetree/bindings/pwm/brcm,kona-pwm.txt
> > > @@ -3,7 +3,7 @@ Broadcom Kona PWM controller device tree bindings
> > >   This controller has 6 channels.
> > >   Required Properties :
> > > -- compatible: should contain "brcm,kona-pwm"
> > > +- compatible: should contain "brcm,kona-pwm" or "brcm,kona-pwm-v2"
> > Is v2 used on a newer generation of kona SoCs? On i.MX these variants
> > are usually named after the first SoC that came with the new variant. Is
> > this sensible here, too?
> 
> It doesn't make as much sense here as different revs of the IP block are
> picked up based on various decisions.
> 
> A new SoC could decide to use an old version.

IMHO this is no reason to not use the name of the oldest SoC with this
variant. I don't know how the SoC names are in the broadcom family, but
if they were (in order of age, oldest first):

	ant
	bear
	crocodile

and ant and crocodile use the same IP block we would have

a) with v1, v2:

	ant:
		compatible = "brcm,kona-pwm-v1";
	bear:
		compatible = "brcm,kona-pwm-v2";
	crocodile:
		compatible = "brcm,kona-pwm-v1";

; and 

b) with the SoC naming:

	ant:
		compatible = "brcm,kona-ant-pwm";
	bear:
		compatible = "brcm,kona-bear-pwm";
	crocodile:
		compatible = "brcm,kona-crocodile-pwm", "brcm,kona-ant-pwm";

(If you want, drop "brcm,kona-crocodile-pwm", but keeping it is more
defensive.) 

I like b) (with "...-crocodile-...") better than a). crocodile using
"...-ant-..." is not more ugly than crocodile using "...-v1". This is
also a tad more robust because if broadcom releases kona-dolphin and
someone finds a minor difference between the IPs used on ant and
crocodile it depends on the order of these events who gets v3, while
with the SoC naming the result is clear.

(OK, and given that "brcm,kona-pwm" is already fixed, both approaches
need slight adaption, but I guess you still get what I meant.)

Best regards
Uwe
Scott Branden Jan. 16, 2019, 12:14 a.m. | #4
Hi Uwe,

On 2019-01-12 7:05 a.m., Uwe Kleine-König wrote:
> Hello Scott,
>
> On Fri, Jan 11, 2019 at 01:28:45PM -0800, Scott Branden wrote:
>> On 2019-01-11 12:48 p.m., Uwe Kleine-König wrote:
>>> On Fri, Jan 11, 2019 at 10:51:14AM +0530, Sheetal Tigadoli wrote:
>>>> From: Praveen Kumar B <praveen.b@broadcom.com>
>>>>
>>>> Add new compatible string for new version of pwm-kona
>>>>
>>>> Signed-off-by: Praveen Kumar B <praveen.b@broadcom.com>
>>>> Reviewed-by: Ray Jui <ray.jui@broadcom.com>
>>>> Reviewed-by: Scott Branden <scott.branden@broadcom.com>
>>>> Signed-off-by: Sheetal Tigadoli <sheetal.tigadoli@broadcom.com>
>>>> ---
>>>>    Documentation/devicetree/bindings/pwm/brcm,kona-pwm.txt | 2 +-
>>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/pwm/brcm,kona-pwm.txt b/Documentation/devicetree/bindings/pwm/brcm,kona-pwm.txt
>>>> index 8eae9fe..d37f697 100644
>>>> --- a/Documentation/devicetree/bindings/pwm/brcm,kona-pwm.txt
>>>> +++ b/Documentation/devicetree/bindings/pwm/brcm,kona-pwm.txt
>>>> @@ -3,7 +3,7 @@ Broadcom Kona PWM controller device tree bindings
>>>>    This controller has 6 channels.
>>>>    Required Properties :
>>>> -- compatible: should contain "brcm,kona-pwm"
>>>> +- compatible: should contain "brcm,kona-pwm" or "brcm,kona-pwm-v2"
>>> Is v2 used on a newer generation of kona SoCs? On i.MX these variants
>>> are usually named after the first SoC that came with the new variant. Is
>>> this sensible here, too?
>> It doesn't make as much sense here as different revs of the IP block are
>> picked up based on various decisions.
>>
>> A new SoC could decide to use an old version.
> IMHO this is no reason to not use the name of the oldest SoC with this
> variant. I don't know how the SoC names are in the broadcom family, but
> if they were (in order of age, oldest first):
>
> 	ant
> 	bear
> 	crocodile
>
> and ant and crocodile use the same IP block we would have
>
> a) with v1, v2:
>
> 	ant:
> 		compatible = "brcm,kona-pwm-v1";
> 	bear:
> 		compatible = "brcm,kona-pwm-v2";
> 	crocodile:
> 		compatible = "brcm,kona-pwm-v1";
>
> ; and
>
> b) with the SoC naming:
>
> 	ant:
> 		compatible = "brcm,kona-ant-pwm";
> 	bear:
> 		compatible = "brcm,kona-bear-pwm";
> 	crocodile:
> 		compatible = "brcm,kona-crocodile-pwm", "brcm,kona-ant-pwm";
>
> (If you want, drop "brcm,kona-crocodile-pwm", but keeping it is more
> defensive.)
>
> I like b) (with "...-crocodile-...") better than a). crocodile using
> "...-ant-..." is not more ugly than crocodile using "...-v1". This is
> also a tad more robust because if broadcom releases kona-dolphin and
> someone finds a minor difference between the IPs used on ant and
> crocodile it depends on the order of these events who gets v3, while
> with the SoC naming the result is clear.
>
> (OK, and given that "brcm,kona-pwm" is already fixed, both approaches
> need slight adaption, but I guess you still get what I meant.)

Thanks for your thoughts and explanation.

It is unfortunate devicetree has no proper guidelines or documentation on

binding naming.  In the interest of getting this upstream we can name it

"brcm, omega-pwm".  We can drop kona from the binding name as that 
architecture

is really no more - only IP derived from it is - hence the name kona-v2 
previously.

>
> Best regards
> Uwe
>
>
Cheers,
Scott
Rob Herring Jan. 21, 2019, 11:11 p.m. | #5
On Tue, Jan 15, 2019 at 04:14:21PM -0800, Scott Branden wrote:
> Hi Uwe,
> 
> On 2019-01-12 7:05 a.m., Uwe Kleine-König wrote:
> > Hello Scott,
> > 
> > On Fri, Jan 11, 2019 at 01:28:45PM -0800, Scott Branden wrote:
> > > On 2019-01-11 12:48 p.m., Uwe Kleine-König wrote:
> > > > On Fri, Jan 11, 2019 at 10:51:14AM +0530, Sheetal Tigadoli wrote:
> > > > > From: Praveen Kumar B <praveen.b@broadcom.com>
> > > > > 
> > > > > Add new compatible string for new version of pwm-kona
> > > > > 
> > > > > Signed-off-by: Praveen Kumar B <praveen.b@broadcom.com>
> > > > > Reviewed-by: Ray Jui <ray.jui@broadcom.com>
> > > > > Reviewed-by: Scott Branden <scott.branden@broadcom.com>
> > > > > Signed-off-by: Sheetal Tigadoli <sheetal.tigadoli@broadcom.com>
> > > > > ---
> > > > >    Documentation/devicetree/bindings/pwm/brcm,kona-pwm.txt | 2 +-
> > > > >    1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > 
> > > > > diff --git a/Documentation/devicetree/bindings/pwm/brcm,kona-pwm.txt b/Documentation/devicetree/bindings/pwm/brcm,kona-pwm.txt
> > > > > index 8eae9fe..d37f697 100644
> > > > > --- a/Documentation/devicetree/bindings/pwm/brcm,kona-pwm.txt
> > > > > +++ b/Documentation/devicetree/bindings/pwm/brcm,kona-pwm.txt
> > > > > @@ -3,7 +3,7 @@ Broadcom Kona PWM controller device tree bindings
> > > > >    This controller has 6 channels.
> > > > >    Required Properties :
> > > > > -- compatible: should contain "brcm,kona-pwm"
> > > > > +- compatible: should contain "brcm,kona-pwm" or "brcm,kona-pwm-v2"
> > > > Is v2 used on a newer generation of kona SoCs? On i.MX these variants
> > > > are usually named after the first SoC that came with the new variant. Is
> > > > this sensible here, too?
> > > It doesn't make as much sense here as different revs of the IP block are
> > > picked up based on various decisions.
> > > 
> > > A new SoC could decide to use an old version.
> > IMHO this is no reason to not use the name of the oldest SoC with this
> > variant. I don't know how the SoC names are in the broadcom family, but
> > if they were (in order of age, oldest first):
> > 
> > 	ant
> > 	bear
> > 	crocodile
> > 
> > and ant and crocodile use the same IP block we would have
> > 
> > a) with v1, v2:
> > 
> > 	ant:
> > 		compatible = "brcm,kona-pwm-v1";
> > 	bear:
> > 		compatible = "brcm,kona-pwm-v2";
> > 	crocodile:
> > 		compatible = "brcm,kona-pwm-v1";

Version numbers can be fine, but generally only as fallbacks as even the 
same IP version can be integrated into an SoC differently.

The other issue with versions is they should be meanful such as 
corresponding to version tags in IP repos. Often, I'd guess anything 
with a 'v1' is just what some s/w person made up. Of course, we only 
can really know that for opensource IP or programmable logic IP.

If you do use versions, document what the versioning scheme is.

> > 
> > ; and
> > 
> > b) with the SoC naming:
> > 
> > 	ant:
> > 		compatible = "brcm,kona-ant-pwm";
> > 	bear:
> > 		compatible = "brcm,kona-bear-pwm";
> > 	crocodile:
> > 		compatible = "brcm,kona-crocodile-pwm", "brcm,kona-ant-pwm";

This is the recommended practice.

> > 
> > (If you want, drop "brcm,kona-crocodile-pwm", but keeping it is more
> > defensive.)

Generally, you should have "brcm,kona-crocodile-pwm" in case there's 
some difference found later. Then you can support the bug or feature 
without a DT change.

> > 
> > I like b) (with "...-crocodile-...") better than a). crocodile using
> > "...-ant-..." is not more ugly than crocodile using "...-v1". This is
> > also a tad more robust because if broadcom releases kona-dolphin and
> > someone finds a minor difference between the IPs used on ant and
> > crocodile it depends on the order of these events who gets v3, while
> > with the SoC naming the result is clear.
> > 
> > (OK, and given that "brcm,kona-pwm" is already fixed, both approaches
> > need slight adaption, but I guess you still get what I meant.)
> 
> Thanks for your thoughts and explanation.
> 
> It is unfortunate devicetree has no proper guidelines or documentation on
> 
> binding naming.  In the interest of getting this upstream we can name it

Surely we've captured that somewhere...

> 
> "brcm, omega-pwm".  We can drop kona from the binding name as that
> architecture
> 
> is really no more - only IP derived from it is - hence the name kona-v2
> previously.
> 
> > 
> > Best regards
> > Uwe
> > 
> > 
> Cheers,
> Scott
Scott Branden Jan. 22, 2019, 8:12 p.m. | #6
Hi Rob,

On 2019-01-21 3:11 p.m., Rob Herring wrote:
> On Tue, Jan 15, 2019 at 04:14:21PM -0800, Scott Branden wrote:
>> Hi Uwe,
>>
>> On 2019-01-12 7:05 a.m., Uwe Kleine-König wrote:
>>> Hello Scott,
>>>
>>> On Fri, Jan 11, 2019 at 01:28:45PM -0800, Scott Branden wrote:
>>>> On 2019-01-11 12:48 p.m., Uwe Kleine-König wrote:
>>>>> On Fri, Jan 11, 2019 at 10:51:14AM +0530, Sheetal Tigadoli wrote:
>>>>>> From: Praveen Kumar B <praveen.b@broadcom.com>
>>>>>>
>>>>>> Add new compatible string for new version of pwm-kona
>>>>>>
>>>>>> Signed-off-by: Praveen Kumar B <praveen.b@broadcom.com>
>>>>>> Reviewed-by: Ray Jui <ray.jui@broadcom.com>
>>>>>> Reviewed-by: Scott Branden <scott.branden@broadcom.com>
>>>>>> Signed-off-by: Sheetal Tigadoli <sheetal.tigadoli@broadcom.com>
>>>>>> ---
>>>>>>     Documentation/devicetree/bindings/pwm/brcm,kona-pwm.txt | 2 +-
>>>>>>     1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/Documentation/devicetree/bindings/pwm/brcm,kona-pwm.txt b/Documentation/devicetree/bindings/pwm/brcm,kona-pwm.txt
>>>>>> index 8eae9fe..d37f697 100644
>>>>>> --- a/Documentation/devicetree/bindings/pwm/brcm,kona-pwm.txt
>>>>>> +++ b/Documentation/devicetree/bindings/pwm/brcm,kona-pwm.txt
>>>>>> @@ -3,7 +3,7 @@ Broadcom Kona PWM controller device tree bindings
>>>>>>     This controller has 6 channels.
>>>>>>     Required Properties :
>>>>>> -- compatible: should contain "brcm,kona-pwm"
>>>>>> +- compatible: should contain "brcm,kona-pwm" or "brcm,kona-pwm-v2"
>>>>> Is v2 used on a newer generation of kona SoCs? On i.MX these variants
>>>>> are usually named after the first SoC that came with the new variant. Is
>>>>> this sensible here, too?
>>>> It doesn't make as much sense here as different revs of the IP block are
>>>> picked up based on various decisions.
>>>>
>>>> A new SoC could decide to use an old version.
>>> IMHO this is no reason to not use the name of the oldest SoC with this
>>> variant. I don't know how the SoC names are in the broadcom family, but
>>> if they were (in order of age, oldest first):
>>>
>>> 	ant
>>> 	bear
>>> 	crocodile
>>>
>>> and ant and crocodile use the same IP block we would have
>>>
>>> a) with v1, v2:
>>>
>>> 	ant:
>>> 		compatible = "brcm,kona-pwm-v1";
>>> 	bear:
>>> 		compatible = "brcm,kona-pwm-v2";
>>> 	crocodile:
>>> 		compatible = "brcm,kona-pwm-v1";
> Version numbers can be fine, but generally only as fallbacks as even the
> same IP version can be integrated into an SoC differently.
>
> The other issue with versions is they should be meanful such as
> corresponding to version tags in IP repos. Often, I'd guess anything
> with a 'v1' is just what some s/w person made up. Of course, we only
> can really know that for opensource IP or programmable logic IP.
>
> If you do use versions, document what the versioning scheme is.
>
>>> ; and
>>>
>>> b) with the SoC naming:
>>>
>>> 	ant:
>>> 		compatible = "brcm,kona-ant-pwm";
>>> 	bear:
>>> 		compatible = "brcm,kona-bear-pwm";
>>> 	crocodile:
>>> 		compatible = "brcm,kona-crocodile-pwm", "brcm,kona-ant-pwm";
> This is the recommended practice.
>
>>> (If you want, drop "brcm,kona-crocodile-pwm", but keeping it is more
>>> defensive.)
> Generally, you should have "brcm,kona-crocodile-pwm" in case there's
> some difference found later. Then you can support the bug or feature
> without a DT change.

No DT change would be necessary in any case.

A check against the SOC type in the driver without additional DT 
compatibility strings could be done.

>
>>> I like b) (with "...-crocodile-...") better than a). crocodile using
>>> "...-ant-..." is not more ugly than crocodile using "...-v1". This is
>>> also a tad more robust because if broadcom releases kona-dolphin and
>>> someone finds a minor difference between the IPs used on ant and
>>> crocodile it depends on the order of these events who gets v3, while
>>> with the SoC naming the result is clear.
>>>
>>> (OK, and given that "brcm,kona-pwm" is already fixed, both approaches
>>> need slight adaption, but I guess you still get what I meant.)
>> Thanks for your thoughts and explanation.
>>
>> It is unfortunate devicetree has no proper guidelines or documentation on
>>
>> binding naming.  In the interest of getting this upstream we can name it
> Surely we've captured that somewhere...

Please point me at such documentation.

There is no consistency in kernel drivers from what I have seen.

>
>> "brcm, omega-pwm".  We can drop kona from the binding name as that
>> architecture
>>
>> is really no more - only IP derived from it is - hence the name kona-v2
>> previously.
>>
>>> Best regards
>>> Uwe
>>>
>>>
>> Cheers,
>> Scott

Patch

diff --git a/Documentation/devicetree/bindings/pwm/brcm,kona-pwm.txt b/Documentation/devicetree/bindings/pwm/brcm,kona-pwm.txt
index 8eae9fe..d37f697 100644
--- a/Documentation/devicetree/bindings/pwm/brcm,kona-pwm.txt
+++ b/Documentation/devicetree/bindings/pwm/brcm,kona-pwm.txt
@@ -3,7 +3,7 @@  Broadcom Kona PWM controller device tree bindings
 This controller has 6 channels.
 
 Required Properties :
-- compatible: should contain "brcm,kona-pwm"
+- compatible: should contain "brcm,kona-pwm" or "brcm,kona-pwm-v2"
 - reg: physical base address and length of the controller's registers
 - clocks: phandle + clock specifier pair for the external clock
 - #pwm-cells: Should be 3. See pwm.txt in this directory for a