| Message ID | b8101fd6d303c1993ee294eeee8bce4d24b9547f.1550061925.git.baolin.wang@linaro.org |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | [1/2] dt-bindings: gpio: Use SoC compatible string instead of wildcard string | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success |
śr., 13 lut 2019 o 13:49 Baolin Wang <baolin.wang@linaro.org> napisał(a): > > Use SoC compatible string instead of wildcard string. > > Signed-off-by: Baolin Wang <baolin.wang@linaro.org> > --- > .../devicetree/bindings/gpio/gpio-eic-sprd.txt | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/Documentation/devicetree/bindings/gpio/gpio-eic-sprd.txt b/Documentation/devicetree/bindings/gpio/gpio-eic-sprd.txt > index 93d98d0..54040a2 100644 > --- a/Documentation/devicetree/bindings/gpio/gpio-eic-sprd.txt > +++ b/Documentation/devicetree/bindings/gpio/gpio-eic-sprd.txt > @@ -33,7 +33,7 @@ Required properties: > "sprd,sc9860-eic-latch", > "sprd,sc9860-eic-async", > "sprd,sc9860-eic-sync", > - "sprd,sc27xx-eic". > + "sprd,sc2731-eic". > - reg: Define the base and range of the I/O address space containing > the GPIO controller registers. > - gpio-controller: Marks the device node as a GPIO controller. > @@ -86,7 +86,7 @@ Example: > }; > > pmic_eic: gpio@300 { > - compatible = "sprd,sc27xx-eic"; > + compatible = "sprd,sc2731-eic"; > reg = <0x300>; > interrupt-parent = <&sc2731_pmic>; > interrupts = <5 IRQ_TYPE_LEVEL_HIGH>; > -- > 1.7.9.5 > Hi, please indicate the module this concerns in the subject line. Something like: "dt-bindings: gpio: sprd: ..." Bartosz
śr., 13 lut 2019 o 13:49 Baolin Wang <baolin.wang@linaro.org> napisał(a): > > Change to use SoC compatible string instead of wildcard string. > > Signed-off-by: Baolin Wang <baolin.wang@linaro.org> > --- > drivers/gpio/gpio-pmic-eic-sprd.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpio/gpio-pmic-eic-sprd.c b/drivers/gpio/gpio-pmic-eic-sprd.c > index ac573da..24228cf 100644 > --- a/drivers/gpio/gpio-pmic-eic-sprd.c > +++ b/drivers/gpio/gpio-pmic-eic-sprd.c > @@ -364,7 +364,7 @@ static int sprd_pmic_eic_probe(struct platform_device *pdev) > } > > static const struct of_device_id sprd_pmic_eic_of_match[] = { > - { .compatible = "sprd,sc27xx-eic", }, > + { .compatible = "sprd,sc2731-eic", }, > { /* end of list */ } > }; > MODULE_DEVICE_TABLE(of, sprd_pmic_eic_of_match); > -- > 1.7.9.5 > We guarantee to make older device-trees to work with new kernel so you can add the new compatible, but you can't remove the old one. Bart
On Wed, 13 Feb 2019 at 20:57, Bartosz Golaszewski <bgolaszewski@baylibre.com> wrote: > > śr., 13 lut 2019 o 13:49 Baolin Wang <baolin.wang@linaro.org> napisał(a): > > > > Use SoC compatible string instead of wildcard string. > > > > Signed-off-by: Baolin Wang <baolin.wang@linaro.org> > > --- > > .../devicetree/bindings/gpio/gpio-eic-sprd.txt | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/Documentation/devicetree/bindings/gpio/gpio-eic-sprd.txt b/Documentation/devicetree/bindings/gpio/gpio-eic-sprd.txt > > index 93d98d0..54040a2 100644 > > --- a/Documentation/devicetree/bindings/gpio/gpio-eic-sprd.txt > > +++ b/Documentation/devicetree/bindings/gpio/gpio-eic-sprd.txt > > @@ -33,7 +33,7 @@ Required properties: > > "sprd,sc9860-eic-latch", > > "sprd,sc9860-eic-async", > > "sprd,sc9860-eic-sync", > > - "sprd,sc27xx-eic". > > + "sprd,sc2731-eic". > > - reg: Define the base and range of the I/O address space containing > > the GPIO controller registers. > > - gpio-controller: Marks the device node as a GPIO controller. > > @@ -86,7 +86,7 @@ Example: > > }; > > > > pmic_eic: gpio@300 { > > - compatible = "sprd,sc27xx-eic"; > > + compatible = "sprd,sc2731-eic"; > > reg = <0x300>; > > interrupt-parent = <&sc2731_pmic>; > > interrupts = <5 IRQ_TYPE_LEVEL_HIGH>; > > -- > > 1.7.9.5 > > > > Hi, > > please indicate the module this concerns in the subject line. > Something like: "dt-bindings: gpio: sprd: ..." Sure. Thanks.
On Wed, 13 Feb 2019 at 20:59, Bartosz Golaszewski <bgolaszewski@baylibre.com> wrote: > > śr., 13 lut 2019 o 13:49 Baolin Wang <baolin.wang@linaro.org> napisał(a): > > > > Change to use SoC compatible string instead of wildcard string. > > > > Signed-off-by: Baolin Wang <baolin.wang@linaro.org> > > --- > > drivers/gpio/gpio-pmic-eic-sprd.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/gpio/gpio-pmic-eic-sprd.c b/drivers/gpio/gpio-pmic-eic-sprd.c > > index ac573da..24228cf 100644 > > --- a/drivers/gpio/gpio-pmic-eic-sprd.c > > +++ b/drivers/gpio/gpio-pmic-eic-sprd.c > > @@ -364,7 +364,7 @@ static int sprd_pmic_eic_probe(struct platform_device *pdev) > > } > > > > static const struct of_device_id sprd_pmic_eic_of_match[] = { > > - { .compatible = "sprd,sc27xx-eic", }, > > + { .compatible = "sprd,sc2731-eic", }, > > { /* end of list */ } > > }; > > MODULE_DEVICE_TABLE(of, sprd_pmic_eic_of_match); > > -- > > 1.7.9.5 > > > > We guarantee to make older device-trees to work with new kernel so you > can add the new compatible, but you can't remove the old one. But the old one is incorrect, and we still keep it?
śr., 13 lut 2019 o 14:15 Baolin Wang <baolin.wang@linaro.org> napisał(a): > > On Wed, 13 Feb 2019 at 20:59, Bartosz Golaszewski > <bgolaszewski@baylibre.com> wrote: > > > > śr., 13 lut 2019 o 13:49 Baolin Wang <baolin.wang@linaro.org> napisał(a): > > > > > > Change to use SoC compatible string instead of wildcard string. > > > > > > Signed-off-by: Baolin Wang <baolin.wang@linaro.org> > > > --- > > > drivers/gpio/gpio-pmic-eic-sprd.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/drivers/gpio/gpio-pmic-eic-sprd.c b/drivers/gpio/gpio-pmic-eic-sprd.c > > > index ac573da..24228cf 100644 > > > --- a/drivers/gpio/gpio-pmic-eic-sprd.c > > > +++ b/drivers/gpio/gpio-pmic-eic-sprd.c > > > @@ -364,7 +364,7 @@ static int sprd_pmic_eic_probe(struct platform_device *pdev) > > > } > > > > > > static const struct of_device_id sprd_pmic_eic_of_match[] = { > > > - { .compatible = "sprd,sc27xx-eic", }, > > > + { .compatible = "sprd,sc2731-eic", }, > > > { /* end of list */ } > > > }; > > > MODULE_DEVICE_TABLE(of, sprd_pmic_eic_of_match); > > > -- > > > 1.7.9.5 > > > > > > > We guarantee to make older device-trees to work with new kernel so you > > can add the new compatible, but you can't remove the old one. > > But the old one is incorrect, and we still keep it? > Well in theory the device-tree is supposed to be a stable ABI so once it's released, it should work with any following kernel version. In practice changes are sometimes allowed and there are also bugs in DT files. Linus: what do you think? Bart
On Wed, Feb 13, 2019 at 5:10 PM Bartosz Golaszewski <bgolaszewski@baylibre.com> wrote: > śr., 13 lut 2019 o 14:15 Baolin Wang <baolin.wang@linaro.org> napisał(a): > > > > On Wed, 13 Feb 2019 at 20:59, Bartosz Golaszewski > > <bgolaszewski@baylibre.com> wrote: > > > > > > śr., 13 lut 2019 o 13:49 Baolin Wang <baolin.wang@linaro.org> napisał(a): > > > > > > > > Change to use SoC compatible string instead of wildcard string. > > > > > > > > Signed-off-by: Baolin Wang <baolin.wang@linaro.org> > > > > --- > > > > drivers/gpio/gpio-pmic-eic-sprd.c | 2 +- > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > diff --git a/drivers/gpio/gpio-pmic-eic-sprd.c b/drivers/gpio/gpio-pmic-eic-sprd.c > > > > index ac573da..24228cf 100644 > > > > --- a/drivers/gpio/gpio-pmic-eic-sprd.c > > > > +++ b/drivers/gpio/gpio-pmic-eic-sprd.c > > > > @@ -364,7 +364,7 @@ static int sprd_pmic_eic_probe(struct platform_device *pdev) > > > > } > > > > > > > > static const struct of_device_id sprd_pmic_eic_of_match[] = { > > > > - { .compatible = "sprd,sc27xx-eic", }, > > > > + { .compatible = "sprd,sc2731-eic", }, > > > > { /* end of list */ } > > > > }; > > > > MODULE_DEVICE_TABLE(of, sprd_pmic_eic_of_match); > > > > -- > > > > 1.7.9.5 > > > > > > > > > > We guarantee to make older device-trees to work with new kernel so you > > > can add the new compatible, but you can't remove the old one. > > > > But the old one is incorrect, and we still keep it? > > > > Well in theory the device-tree is supposed to be a stable ABI so once > it's released, it should work with any following kernel version. > > In practice changes are sometimes allowed and there are also bugs in DT files. > > Linus: what do you think? In this specific case I'd keep both strings, it doesn't hurt does it? You could add a comment to the wildcard string saying it is only there for compatibility with elder device trees. In general as long as there are not (a lot of) products shipped with a certain device tree, I don't care much whether we change the bindings or contents. The hard rule to keep the device trees backward-compatible comes from SPARC SunOS where the DTB was burned into a BIOS ROM that was hard or impossible to update, Linux just had to handle whatever was in there. If the situation with the device tree we change is not similiar, we should not care either. In practice there are companies and developers that always recompile and ship their device trees at the same time as they compile and ship their kernel, and in that case we need not care about backward compatibility. While the device tree enablement on ARM started out with the former (strict) assumption, the practice of using DTs has shown that it is an unrealistic and inappropriate stance to have for all device trees. (IMO!) So I don't mind if you break compatibility here. Yours, Linus Walleij
On Thu, 14 Feb 2019 at 15:56, Linus Walleij <linus.walleij@linaro.org> wrote: > > On Wed, Feb 13, 2019 at 5:10 PM Bartosz Golaszewski > <bgolaszewski@baylibre.com> wrote: > > śr., 13 lut 2019 o 14:15 Baolin Wang <baolin.wang@linaro.org> napisał(a): > > > > > > On Wed, 13 Feb 2019 at 20:59, Bartosz Golaszewski > > > <bgolaszewski@baylibre.com> wrote: > > > > > > > > śr., 13 lut 2019 o 13:49 Baolin Wang <baolin.wang@linaro.org> napisał(a): > > > > > > > > > > Change to use SoC compatible string instead of wildcard string. > > > > > > > > > > Signed-off-by: Baolin Wang <baolin.wang@linaro.org> > > > > > --- > > > > > drivers/gpio/gpio-pmic-eic-sprd.c | 2 +- > > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > > > diff --git a/drivers/gpio/gpio-pmic-eic-sprd.c b/drivers/gpio/gpio-pmic-eic-sprd.c > > > > > index ac573da..24228cf 100644 > > > > > --- a/drivers/gpio/gpio-pmic-eic-sprd.c > > > > > +++ b/drivers/gpio/gpio-pmic-eic-sprd.c > > > > > @@ -364,7 +364,7 @@ static int sprd_pmic_eic_probe(struct platform_device *pdev) > > > > > } > > > > > > > > > > static const struct of_device_id sprd_pmic_eic_of_match[] = { > > > > > - { .compatible = "sprd,sc27xx-eic", }, > > > > > + { .compatible = "sprd,sc2731-eic", }, > > > > > { /* end of list */ } > > > > > }; > > > > > MODULE_DEVICE_TABLE(of, sprd_pmic_eic_of_match); > > > > > -- > > > > > 1.7.9.5 > > > > > > > > > > > > > We guarantee to make older device-trees to work with new kernel so you > > > > can add the new compatible, but you can't remove the old one. > > > > > > But the old one is incorrect, and we still keep it? > > > > > > > Well in theory the device-tree is supposed to be a stable ABI so once > > it's released, it should work with any following kernel version. > > > > In practice changes are sometimes allowed and there are also bugs in DT files. > > > > Linus: what do you think? > > In this specific case I'd keep both strings, it doesn't hurt does it? > > You could add a comment to the wildcard string saying it is only there > for compatibility with elder device trees. > > In general as long as there are not (a lot of) products shipped with > a certain device tree, I don't care much whether we change the bindings > or contents. > > The hard rule to keep the device trees backward-compatible comes > from SPARC SunOS where the DTB was burned into a BIOS ROM > that was hard or impossible to update, Linux just had to handle > whatever was in there. If the situation with the device tree we change > is not similiar, we should not care either. > > In practice there are companies and developers that always > recompile and ship their device trees at the same time as they > compile and ship their kernel, and in that case we need not care > about backward compatibility. > > While the device tree enablement on ARM started out with the > former (strict) assumption, the practice of using DTs has shown > that it is an unrealistic and inappropriate stance to have for all > device trees. (IMO!) So I don't mind if you break compatibility here. > Thanks for your explanation. Yes, as our dts and drivers development are still in progress, I do not think we need care about the backward compatibility issue. So I still intend to remove the incorrect wildcard string.
diff --git a/Documentation/devicetree/bindings/gpio/gpio-eic-sprd.txt b/Documentation/devicetree/bindings/gpio/gpio-eic-sprd.txt index 93d98d0..54040a2 100644 --- a/Documentation/devicetree/bindings/gpio/gpio-eic-sprd.txt +++ b/Documentation/devicetree/bindings/gpio/gpio-eic-sprd.txt @@ -33,7 +33,7 @@ Required properties: "sprd,sc9860-eic-latch", "sprd,sc9860-eic-async", "sprd,sc9860-eic-sync", - "sprd,sc27xx-eic". + "sprd,sc2731-eic". - reg: Define the base and range of the I/O address space containing the GPIO controller registers. - gpio-controller: Marks the device node as a GPIO controller. @@ -86,7 +86,7 @@ Example: }; pmic_eic: gpio@300 { - compatible = "sprd,sc27xx-eic"; + compatible = "sprd,sc2731-eic"; reg = <0x300>; interrupt-parent = <&sc2731_pmic>; interrupts = <5 IRQ_TYPE_LEVEL_HIGH>;
Use SoC compatible string instead of wildcard string. Signed-off-by: Baolin Wang <baolin.wang@linaro.org> --- .../devicetree/bindings/gpio/gpio-eic-sprd.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)