| Message ID | 7e1de17a43f2cde73aecd2773e2b3a5fdfd294f3.1547781007.git.ryder.lee@mediatek.com |
|---|---|
| State | Rejected |
| Headers | show |
| Series | [v1,1/5] pwm: mediatek: add a property "mediatek,num-pwms" | expand |
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
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
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
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
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>; };