| Message ID | 1561435529-7835-2-git-send-email-bdodge09@gmail.com |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | fix vendor prefix for arcxcnn driver and bindings | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success |
On Tue, Jun 25, 2019 at 12:05:28AM -0400, Brian Dodge wrote: > The vendor-prefixes.txt file properly refers to ArcticSand > as arctic but the driver bindings improperly abbreviated the > prefix to arc. This was a mistake in the original patch > > Signed-off-by: Brian Dodge <bdodge09@gmail.com> > --- > .../bindings/leds/backlight/arcxcnn_bl.txt | 24 +++++++++++++--------- > 1 file changed, 14 insertions(+), 10 deletions(-) > > diff --git a/Documentation/devicetree/bindings/leds/backlight/arcxcnn_bl.txt b/Documentation/devicetree/bindings/leds/backlight/arcxcnn_bl.txt > index 230abde..9cf4c44 100644 > --- a/Documentation/devicetree/bindings/leds/backlight/arcxcnn_bl.txt > +++ b/Documentation/devicetree/bindings/leds/backlight/arcxcnn_bl.txt > @@ -1,8 +1,12 @@ > -Binding for ArcticSand arc2c0608 LED driver > +Binding for ArcticSand arc family LED drivers > > Required properties: > -- compatible: should be "arc,arc2c0608" > -- reg: slave address > +- compatible: one of > + "arctic,arc1c0608" > + "arctic,arc2c0608" > + "arctic,arc3c0845" This is more a question for the DT folks than for Brian but... AFAICT this patch is fixing the binding for the ArcticSand devices to use the correct value from vendor-prefixes.yaml and has been previously discussed here: https://lkml.org/lkml/2018/9/25/726 Currently this patch series just updates the DT bindings but the implementation also honours the old values (since there is a Chromebook in the wild that uses the current bindings). Hence I'm not clear whether the bindings should document the deprecated options too (e.g. make it easier to find the bindings doc with git grep and friends). Daniel. > + > +- reg: slave address > > Optional properties: > - default-brightness: brightness value on boot, value from: 0-4095 > @@ -11,19 +15,19 @@ Optional properties: > - led-sources: List of enabled channels from 0 to 5. > See Documentation/devicetree/bindings/leds/common.txt > > -- arc,led-config-0: setting for register ILED_CONFIG_0 > -- arc,led-config-1: setting for register ILED_CONFIG_1 > -- arc,dim-freq: PWM mode frequence setting (bits [3:0] used) > -- arc,comp-config: setting for register CONFIG_COMP > -- arc,filter-config: setting for register FILTER_CONFIG > -- arc,trim-config: setting for register IMAXTUNE > +- arctic,led-config-0: setting for register ILED_CONFIG_0 > +- arctic,led-config-1: setting for register ILED_CONFIG_1 > +- arctic,dim-freq: PWM mode frequence setting (bits [3:0] used) > +- arctic,comp-config: setting for register CONFIG_COMP > +- arctic,filter-config: setting for register FILTER_CONFIG > +- arctic,trim-config: setting for register IMAXTUNE > > Note: Optional properties not specified will default to values in IC EPROM > > Example: > > arc2c0608@30 { > - compatible = "arc,arc2c0608"; > + compatible = "arctic,arc2c0608"; > reg = <0x30>; > default-brightness = <500>; > label = "lcd-backlight"; > -- > 2.7.4 >
On Tue, Jun 25, 2019 at 07:44:06AM -0400, Brian Dodge wrote: > I would like to deprecate the old prefix in the future after communicating > with all chip customers, which is why the old prefix is not documented in > the new bindings. Deprecation is fine (by me at least) it's just that I'm not sure that removing the documentation for the deprecated bindings is the right way to do it. What is the prior art here? Daniel. > > > On Tue, Jun 25, 2019, 4:55 AM Daniel Thompson <daniel.thompson@linaro.org> > wrote: > > > On Tue, Jun 25, 2019 at 12:05:28AM -0400, Brian Dodge wrote: > > > The vendor-prefixes.txt file properly refers to ArcticSand > > > as arctic but the driver bindings improperly abbreviated the > > > prefix to arc. This was a mistake in the original patch > > > > > > Signed-off-by: Brian Dodge <bdodge09@gmail.com> > > > --- > > > .../bindings/leds/backlight/arcxcnn_bl.txt | 24 > > +++++++++++++--------- > > > 1 file changed, 14 insertions(+), 10 deletions(-) > > > > > > diff --git > > a/Documentation/devicetree/bindings/leds/backlight/arcxcnn_bl.txt > > b/Documentation/devicetree/bindings/leds/backlight/arcxcnn_bl.txt > > > index 230abde..9cf4c44 100644 > > > --- a/Documentation/devicetree/bindings/leds/backlight/arcxcnn_bl.txt > > > +++ b/Documentation/devicetree/bindings/leds/backlight/arcxcnn_bl.txt > > > @@ -1,8 +1,12 @@ > > > -Binding for ArcticSand arc2c0608 LED driver > > > +Binding for ArcticSand arc family LED drivers > > > > > > Required properties: > > > -- compatible: should be "arc,arc2c0608" > > > -- reg: slave address > > > +- compatible: one of > > > + "arctic,arc1c0608" > > > + "arctic,arc2c0608" > > > + "arctic,arc3c0845" > > > > This is more a question for the DT folks than for Brian but... > > > > AFAICT this patch is fixing the binding for the ArcticSand devices to > > use the correct value from vendor-prefixes.yaml and has been previously > > discussed here: > > https://lkml.org/lkml/2018/9/25/726 > > > > Currently this patch series just updates the DT bindings but the > > implementation also honours the old values (since there is a Chromebook > > in the wild that uses the current bindings). > > > > Hence I'm not clear whether the bindings should document the deprecated > > options too (e.g. make it easier to find the bindings doc with git grep > > and friends). > > > > > > Daniel. > > > > > > > + > > > +- reg: slave address > > > > > > Optional properties: > > > - default-brightness: brightness value on boot, value from: > > 0-4095 > > > @@ -11,19 +15,19 @@ Optional properties: > > > - led-sources: List of enabled channels from 0 to 5. > > > See > > Documentation/devicetree/bindings/leds/common.txt > > > > > > -- arc,led-config-0: setting for register ILED_CONFIG_0 > > > -- arc,led-config-1: setting for register ILED_CONFIG_1 > > > -- arc,dim-freq: PWM mode frequence setting (bits [3:0] > > used) > > > -- arc,comp-config: setting for register CONFIG_COMP > > > -- arc,filter-config: setting for register FILTER_CONFIG > > > -- arc,trim-config: setting for register IMAXTUNE > > > +- arctic,led-config-0: setting for register ILED_CONFIG_0 > > > +- arctic,led-config-1: setting for register ILED_CONFIG_1 > > > +- arctic,dim-freq: PWM mode frequence setting (bits [3:0] > > used) > > > +- arctic,comp-config: setting for register CONFIG_COMP > > > +- arctic,filter-config: setting for register FILTER_CONFIG > > > +- arctic,trim-config: setting for register IMAXTUNE > > > > > > Note: Optional properties not specified will default to values in IC > > EPROM > > > > > > Example: > > > > > > arc2c0608@30 { > > > - compatible = "arc,arc2c0608"; > > > + compatible = "arctic,arc2c0608"; > > > reg = <0x30>; > > > default-brightness = <500>; > > > label = "lcd-backlight"; > > > -- > > > 2.7.4 > > > > >
Hello On 6/25/19 3:55 AM, Daniel Thompson wrote: > On Tue, Jun 25, 2019 at 12:05:28AM -0400, Brian Dodge wrote: >> The vendor-prefixes.txt file properly refers to ArcticSand >> as arctic but the driver bindings improperly abbreviated the >> prefix to arc. This was a mistake in the original patch >> >> Signed-off-by: Brian Dodge <bdodge09@gmail.com> >> --- >> .../bindings/leds/backlight/arcxcnn_bl.txt | 24 +++++++++++++--------- >> 1 file changed, 14 insertions(+), 10 deletions(-) >> >> diff --git a/Documentation/devicetree/bindings/leds/backlight/arcxcnn_bl.txt b/Documentation/devicetree/bindings/leds/backlight/arcxcnn_bl.txt >> index 230abde..9cf4c44 100644 >> --- a/Documentation/devicetree/bindings/leds/backlight/arcxcnn_bl.txt >> +++ b/Documentation/devicetree/bindings/leds/backlight/arcxcnn_bl.txt >> @@ -1,8 +1,12 @@ >> -Binding for ArcticSand arc2c0608 LED driver >> +Binding for ArcticSand arc family LED drivers >> >> Required properties: >> -- compatible: should be "arc,arc2c0608" >> -- reg: slave address >> +- compatible: one of >> + "arctic,arc1c0608" >> + "arctic,arc2c0608" >> + "arctic,arc3c0845" > This is more a question for the DT folks than for Brian but... > > AFAICT this patch is fixing the binding for the ArcticSand devices to > use the correct value from vendor-prefixes.yaml and has been previously > discussed here: > https://lkml.org/lkml/2018/9/25/726 > > Currently this patch series just updates the DT bindings but the > implementation also honours the old values (since there is a Chromebook > in the wild that uses the current bindings). > > Hence I'm not clear whether the bindings should document the deprecated > options too (e.g. make it easier to find the bindings doc with git grep > and friends). > > > Daniel. > > >> + >> +- reg: slave address >> >> Optional properties: >> - default-brightness: brightness value on boot, value from: 0-4095 >> @@ -11,19 +15,19 @@ Optional properties: >> - led-sources: List of enabled channels from 0 to 5. >> See Documentation/devicetree/bindings/leds/common.txt >> >> -- arc,led-config-0: setting for register ILED_CONFIG_0 >> -- arc,led-config-1: setting for register ILED_CONFIG_1 >> -- arc,dim-freq: PWM mode frequence setting (bits [3:0] used) >> -- arc,comp-config: setting for register CONFIG_COMP >> -- arc,filter-config: setting for register FILTER_CONFIG >> -- arc,trim-config: setting for register IMAXTUNE IMO I would prefer to keep these and mark them as deprecated since the driver will still honor these properties. Maybe in a Optional Deprecated Properties section in the DT binding. Dan >> +- arctic,led-config-0: setting for register ILED_CONFIG_0 >> +- arctic,led-config-1: setting for register ILED_CONFIG_1 >> +- arctic,dim-freq: PWM mode frequence setting (bits [3:0] used) >> +- arctic,comp-config: setting for register CONFIG_COMP >> +- arctic,filter-config: setting for register FILTER_CONFIG >> +- arctic,trim-config: setting for register IMAXTUNE >> >> Note: Optional properties not specified will default to values in IC EPROM >> >> Example: >> >> arc2c0608@30 { >> - compatible = "arc,arc2c0608"; >> + compatible = "arctic,arc2c0608"; >> reg = <0x30>; >> default-brightness = <500>; >> label = "lcd-backlight"; >> -- >> 2.7.4 >>
On Wed, 26 Jun 2019, Daniel Thompson wrote: > On Tue, Jun 25, 2019 at 07:44:06AM -0400, Brian Dodge wrote: > > I would like to deprecate the old prefix in the future after communicating > > with all chip customers, which is why the old prefix is not documented in > > the new bindings. > > Deprecation is fine (by me at least) it's just that I'm not sure that > removing the documentation for the deprecated bindings is the right way > to do it. What is the prior art here? I'd let Rob et al. make the final decision.
On Wed, Jun 26, 2019 at 4:56 AM Daniel Thompson <daniel.thompson@linaro.org> wrote: > > On Tue, Jun 25, 2019 at 07:44:06AM -0400, Brian Dodge wrote: > > I would like to deprecate the old prefix in the future after communicating > > with all chip customers, which is why the old prefix is not documented in > > the new bindings. > > Deprecation is fine (by me at least) it's just that I'm not sure that > removing the documentation for the deprecated bindings is the right way > to do it. What is the prior art here? Kind of depends on how widely used something is and we've done both ways generally. If there are not any upstream dts files, removal is fine IMO. With schema, there's a 'deprecated' keyword coming in draft8. So we'll have a defined way to keep things documented as needed and also warn about their use. Rob
On Wed 2019-06-26 11:56:14, Daniel Thompson wrote: > On Tue, Jun 25, 2019 at 07:44:06AM -0400, Brian Dodge wrote: > > I would like to deprecate the old prefix in the future after communicating > > with all chip customers, which is why the old prefix is not documented in > > the new bindings. > > Deprecation is fine (by me at least) it's just that I'm not sure that > removing the documentation for the deprecated bindings is the right way > to do it. What is the prior art here? I believe we should keep the docs. Pavel
On 6/26/19 9:56 AM, Pavel Machek wrote: > On Wed 2019-06-26 11:56:14, Daniel Thompson wrote: >> On Tue, Jun 25, 2019 at 07:44:06AM -0400, Brian Dodge wrote: >>> I would like to deprecate the old prefix in the future after communicating >>> with all chip customers, which is why the old prefix is not documented in >>> the new bindings. >> Deprecation is fine (by me at least) it's just that I'm not sure that >> removing the documentation for the deprecated bindings is the right way >> to do it. What is the prior art here? > I believe we should keep the docs. I agree with Pavel on keeping the docs. Keep the doc but mark the properties as deprecated since they are not removed from the code Dan > Pavel
diff --git a/Documentation/devicetree/bindings/leds/backlight/arcxcnn_bl.txt b/Documentation/devicetree/bindings/leds/backlight/arcxcnn_bl.txt index 230abde..9cf4c44 100644 --- a/Documentation/devicetree/bindings/leds/backlight/arcxcnn_bl.txt +++ b/Documentation/devicetree/bindings/leds/backlight/arcxcnn_bl.txt @@ -1,8 +1,12 @@ -Binding for ArcticSand arc2c0608 LED driver +Binding for ArcticSand arc family LED drivers Required properties: -- compatible: should be "arc,arc2c0608" -- reg: slave address +- compatible: one of + "arctic,arc1c0608" + "arctic,arc2c0608" + "arctic,arc3c0845" + +- reg: slave address Optional properties: - default-brightness: brightness value on boot, value from: 0-4095 @@ -11,19 +15,19 @@ Optional properties: - led-sources: List of enabled channels from 0 to 5. See Documentation/devicetree/bindings/leds/common.txt -- arc,led-config-0: setting for register ILED_CONFIG_0 -- arc,led-config-1: setting for register ILED_CONFIG_1 -- arc,dim-freq: PWM mode frequence setting (bits [3:0] used) -- arc,comp-config: setting for register CONFIG_COMP -- arc,filter-config: setting for register FILTER_CONFIG -- arc,trim-config: setting for register IMAXTUNE +- arctic,led-config-0: setting for register ILED_CONFIG_0 +- arctic,led-config-1: setting for register ILED_CONFIG_1 +- arctic,dim-freq: PWM mode frequence setting (bits [3:0] used) +- arctic,comp-config: setting for register CONFIG_COMP +- arctic,filter-config: setting for register FILTER_CONFIG +- arctic,trim-config: setting for register IMAXTUNE Note: Optional properties not specified will default to values in IC EPROM Example: arc2c0608@30 { - compatible = "arc,arc2c0608"; + compatible = "arctic,arc2c0608"; reg = <0x30>; default-brightness = <500>; label = "lcd-backlight";
The vendor-prefixes.txt file properly refers to ArcticSand as arctic but the driver bindings improperly abbreviated the prefix to arc. This was a mistake in the original patch Signed-off-by: Brian Dodge <bdodge09@gmail.com> --- .../bindings/leds/backlight/arcxcnn_bl.txt | 24 +++++++++++++--------- 1 file changed, 14 insertions(+), 10 deletions(-)