[v1,2/5] dt-bindings: pwm: add a property "mediatek,num-pwms"

Message ID 7e1de17a43f2cde73aecd2773e2b3a5fdfd294f3.1547781007.git.ryder.lee@mediatek.com
State New
Headers show
Series
  • [v1,1/5] pwm: mediatek: add a property "mediatek,num-pwms"
Related show

Commit Message

Ryder Lee Jan. 18, 2019, 3:24 a.m.
This adds a property "mediatek,num-pwms" in example so that we could
set the number of PWM channels via device tree.

Signed-off-by: Ryder Lee <ryder.lee@mediatek.com>
Reviewed-by: Matthias Brugger <matthias.bgg@gmail.com>
---
Changes since v1: add a Reviewed-by tag.
---
 Documentation/devicetree/bindings/pwm/pwm-mediatek.txt | 2 ++
 1 file changed, 2 insertions(+)

Comments

Matthias Brugger Jan. 18, 2019, 8:44 a.m. | #1
On 18/01/2019 04:24, Ryder Lee wrote:
> This adds a property "mediatek,num-pwms" in example so that we could
> set the number of PWM channels via device tree.
> 
> Signed-off-by: Ryder Lee <ryder.lee@mediatek.com>
> Reviewed-by: Matthias Brugger <matthias.bgg@gmail.com>
> ---
> Changes since v1: add a Reviewed-by tag.
> ---
>  Documentation/devicetree/bindings/pwm/pwm-mediatek.txt | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/pwm/pwm-mediatek.txt b/Documentation/devicetree/bindings/pwm/pwm-mediatek.txt
> index 991728c..f9e2d1f 100644
> --- a/Documentation/devicetree/bindings/pwm/pwm-mediatek.txt
> +++ b/Documentation/devicetree/bindings/pwm/pwm-mediatek.txt
> @@ -20,6 +20,7 @@ Required properties:
>   - pinctrl-names: Must contain a "default" entry.
>   - pinctrl-0: One property must exist for each entry in pinctrl-names.
>     See pinctrl/pinctrl-bindings.txt for details of the property values.
> + - mediatek,num-pwms: the number of PWM channels.
>  
>  Example:
>  	pwm0: pwm@11006000 {
> @@ -37,4 +38,5 @@ Example:
>  			      "pwm3", "pwm4", "pwm5";
>  		pinctrl-names = "default";
>  		pinctrl-0 = <&pwm0_pins>;
> +		mediatek,num-pwms = <5>;
>  	};
> 

Wasn't there a comment in the last version to use num-pwms instead of
mediatek,num-pwms?

Uwe I think you requested that.

Regards,
Matthias
Uwe Kleine-König Jan. 21, 2019, 8:43 a.m. | #2
On Fri, Jan 18, 2019 at 09:44:49AM +0100, Matthias Brugger wrote:
> 
> 
> On 18/01/2019 04:24, Ryder Lee wrote:
> > This adds a property "mediatek,num-pwms" in example so that we could
> > set the number of PWM channels via device tree.
> > 
> > Signed-off-by: Ryder Lee <ryder.lee@mediatek.com>
> > Reviewed-by: Matthias Brugger <matthias.bgg@gmail.com>
> > ---
> > Changes since v1: add a Reviewed-by tag.
> > ---
> >  Documentation/devicetree/bindings/pwm/pwm-mediatek.txt | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/pwm/pwm-mediatek.txt b/Documentation/devicetree/bindings/pwm/pwm-mediatek.txt
> > index 991728c..f9e2d1f 100644
> > --- a/Documentation/devicetree/bindings/pwm/pwm-mediatek.txt
> > +++ b/Documentation/devicetree/bindings/pwm/pwm-mediatek.txt
> > @@ -20,6 +20,7 @@ Required properties:
> >   - pinctrl-names: Must contain a "default" entry.
> >   - pinctrl-0: One property must exist for each entry in pinctrl-names.
> >     See pinctrl/pinctrl-bindings.txt for details of the property values.
> > + - mediatek,num-pwms: the number of PWM channels.
> >  
> >  Example:
> >  	pwm0: pwm@11006000 {
> > @@ -37,4 +38,5 @@ Example:
> >  			      "pwm3", "pwm4", "pwm5";
> >  		pinctrl-names = "default";
> >  		pinctrl-0 = <&pwm0_pins>;
> > +		mediatek,num-pwms = <5>;
> >  	};
> > 
> 
> Wasn't there a comment in the last version to use num-pwms instead of
> mediatek,num-pwms?
> 
> Uwe I think you requested that.

I think it is sensible, but it needs Thierry's blessing. Unfortunately
he seems busy with other stuff and getting feedback from him needs
patience.

Best regards
Uwe
Rob Herring Feb. 18, 2019, 8:36 p.m. | #3
On Fri, Jan 18, 2019 at 09:44:49AM +0100, Matthias Brugger wrote:
> 
> 
> On 18/01/2019 04:24, Ryder Lee wrote:
> > This adds a property "mediatek,num-pwms" in example so that we could
> > set the number of PWM channels via device tree.
> > 
> > Signed-off-by: Ryder Lee <ryder.lee@mediatek.com>
> > Reviewed-by: Matthias Brugger <matthias.bgg@gmail.com>
> > ---
> > Changes since v1: add a Reviewed-by tag.
> > ---
> >  Documentation/devicetree/bindings/pwm/pwm-mediatek.txt | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/pwm/pwm-mediatek.txt b/Documentation/devicetree/bindings/pwm/pwm-mediatek.txt
> > index 991728c..f9e2d1f 100644
> > --- a/Documentation/devicetree/bindings/pwm/pwm-mediatek.txt
> > +++ b/Documentation/devicetree/bindings/pwm/pwm-mediatek.txt
> > @@ -20,6 +20,7 @@ Required properties:
> >   - pinctrl-names: Must contain a "default" entry.
> >   - pinctrl-0: One property must exist for each entry in pinctrl-names.
> >     See pinctrl/pinctrl-bindings.txt for details of the property values.
> > + - mediatek,num-pwms: the number of PWM channels.
> >  
> >  Example:
> >  	pwm0: pwm@11006000 {
> > @@ -37,4 +38,5 @@ Example:
> >  			      "pwm3", "pwm4", "pwm5";
> >  		pinctrl-names = "default";
> >  		pinctrl-0 = <&pwm0_pins>;
> > +		mediatek,num-pwms = <5>;
> >  	};
> > 
> 
> Wasn't there a comment in the last version to use num-pwms instead of
> mediatek,num-pwms?
> 
> Uwe I think you requested that.

Perhaps, but why is this needed? Typically, this would be implied by the 
compatible or the driver doesn't care and we assume 'pwms' properties 
are not out of range.

Rob
Uwe Kleine-König Feb. 19, 2019, 7:30 a.m. | #4
On Mon, Feb 18, 2019 at 02:36:58PM -0600, Rob Herring wrote:
> On Fri, Jan 18, 2019 at 09:44:49AM +0100, Matthias Brugger wrote:
> > 
> > 
> > On 18/01/2019 04:24, Ryder Lee wrote:
> > > This adds a property "mediatek,num-pwms" in example so that we could
> > > set the number of PWM channels via device tree.
> > > 
> > > Signed-off-by: Ryder Lee <ryder.lee@mediatek.com>
> > > Reviewed-by: Matthias Brugger <matthias.bgg@gmail.com>
> > > ---
> > > Changes since v1: add a Reviewed-by tag.
> > > ---
> > >  Documentation/devicetree/bindings/pwm/pwm-mediatek.txt | 2 ++
> > >  1 file changed, 2 insertions(+)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/pwm/pwm-mediatek.txt b/Documentation/devicetree/bindings/pwm/pwm-mediatek.txt
> > > index 991728c..f9e2d1f 100644
> > > --- a/Documentation/devicetree/bindings/pwm/pwm-mediatek.txt
> > > +++ b/Documentation/devicetree/bindings/pwm/pwm-mediatek.txt
> > > @@ -20,6 +20,7 @@ Required properties:
> > >   - pinctrl-names: Must contain a "default" entry.
> > >   - pinctrl-0: One property must exist for each entry in pinctrl-names.
> > >     See pinctrl/pinctrl-bindings.txt for details of the property values.
> > > + - mediatek,num-pwms: the number of PWM channels.
> > >  
> > >  Example:
> > >  	pwm0: pwm@11006000 {
> > > @@ -37,4 +38,5 @@ Example:
> > >  			      "pwm3", "pwm4", "pwm5";
> > >  		pinctrl-names = "default";
> > >  		pinctrl-0 = <&pwm0_pins>;
> > > +		mediatek,num-pwms = <5>;
> > >  	};
> > > 
> > 
> > Wasn't there a comment in the last version to use num-pwms instead of
> > mediatek,num-pwms?
> > 
> > Uwe I think you requested that.
> 
> Perhaps, but why is this needed? Typically, this would be implied by the 
> compatible or the driver doesn't care and we assume 'pwms' properties 
> are not out of range.

I think it is sensible to standardize such a property because there are
a few drivers that support different variants of PWMs that only differ
in the number of supported pwms. See for example pwm-mediatek and
pwm-sun4i. I wrote a longer motivation in a mail that is available at
https://www.spinics.net/lists/linux-pwm/msg08859.html.

The short version is that there are at least four drivers that have to
solve the same task to determine the number of available pwms from the
compatible. While this isn't a hard task it seems sensible to me to
describe this generic property of a piece of hardware that provides PWMs
in the device tree instead of implementing a table in the driver.

Best regards
Uwe

Patch

diff --git a/Documentation/devicetree/bindings/pwm/pwm-mediatek.txt b/Documentation/devicetree/bindings/pwm/pwm-mediatek.txt
index 991728c..f9e2d1f 100644
--- a/Documentation/devicetree/bindings/pwm/pwm-mediatek.txt
+++ b/Documentation/devicetree/bindings/pwm/pwm-mediatek.txt
@@ -20,6 +20,7 @@  Required properties:
  - pinctrl-names: Must contain a "default" entry.
  - pinctrl-0: One property must exist for each entry in pinctrl-names.
    See pinctrl/pinctrl-bindings.txt for details of the property values.
+ - mediatek,num-pwms: the number of PWM channels.
 
 Example:
 	pwm0: pwm@11006000 {
@@ -37,4 +38,5 @@  Example:
 			      "pwm3", "pwm4", "pwm5";
 		pinctrl-names = "default";
 		pinctrl-0 = <&pwm0_pins>;
+		mediatek,num-pwms = <5>;
 	};