| Message ID | 1381765354-28440-2-git-send-email-valentine.barshak@cogentembedded.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
On Mon, Oct 14, 2013 at 04:42:33PM +0100, Valentine Barshak wrote: > This converts the R-Car SATA DT compatibility string to > the <unit>-<soc> format which is the preferred one for > all SH-Mobile devices. The DT bindings are documented. > > Signed-off-by: Valentine Barshak <valentine.barshak@cogentembedded.com> > --- > Documentation/devicetree/bindings/ata/sata_rcar.txt | 16 ++++++++++++++++ > arch/arm/boot/dts/r8a7779.dtsi | 2 +- > drivers/ata/sata_rcar.c | 2 +- > 3 files changed, 18 insertions(+), 2 deletions(-) > create mode 100644 Documentation/devicetree/bindings/ata/sata_rcar.txt > > diff --git a/Documentation/devicetree/bindings/ata/sata_rcar.txt b/Documentation/devicetree/bindings/ata/sata_rcar.txt > new file mode 100644 > index 0000000..2465183 > --- /dev/null > +++ b/Documentation/devicetree/bindings/ata/sata_rcar.txt > @@ -0,0 +1,16 @@ > +* Renesas R-Car SATA > + > +Required properties: > +- compatible : must be "renesas,sata-r8a7779" s/must be/should include/ > +- reg : address range of the SATA registers. Also their size... > +- interrupt-parent : interrupt parent controller phandle This isn't required as such, as the interrupt-controller can be inherited from the parent. As it's a standard auxiliary property I don't think it needs to be documented here. > +- interrupts : must consist of one interrupt specifier. Is there only one interrupt generated by the device? > + > +Example: > + > +sata: sata@fc600000 { > + compatible = "renesas,sata-r8a7779"; > + reg = <0xfc600000 0x2000>; > + interrupt-parent = <&gic>; > + interrupts = <0 100 0x4>; > +}; > diff --git a/arch/arm/boot/dts/r8a7779.dtsi b/arch/arm/boot/dts/r8a7779.dtsi > index da61d27..b1747f5 100644 > --- a/arch/arm/boot/dts/r8a7779.dtsi > +++ b/arch/arm/boot/dts/r8a7779.dtsi > @@ -201,7 +201,7 @@ > }; > > sata: sata@fc600000 { > - compatible = "renesas,rcar-sata"; > + compatible = "renesas,sata-r8a7779"; > reg = <0xfc600000 0x2000>; > interrupt-parent = <&gic>; > interrupts = <0 100 0x4>; > diff --git a/drivers/ata/sata_rcar.c b/drivers/ata/sata_rcar.c > index c2d95e9..c4cd738 100644 > --- a/drivers/ata/sata_rcar.c > +++ b/drivers/ata/sata_rcar.c > @@ -893,7 +893,7 @@ static const struct dev_pm_ops sata_rcar_pm_ops = { > #endif > > static struct of_device_id sata_rcar_match[] = { > - { .compatible = "renesas,rcar-sata", }, > + { .compatible = "renesas,sata-r8a7779", }, > {}, > }; Why is an existing compatible string being removed? Please do not change existing strings, it will break any dts files that are already in use. While the new compatible string may be preferred, the old string should be kept for compatibility. Thanks, Mark. -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Mark, thanks for looking at this. On 10/14/2013 08:13 PM, Mark Rutland wrote: > On Mon, Oct 14, 2013 at 04:42:33PM +0100, Valentine Barshak wrote: >> This converts the R-Car SATA DT compatibility string to >> the <unit>-<soc> format which is the preferred one for >> all SH-Mobile devices. The DT bindings are documented. >> >> Signed-off-by: Valentine Barshak <valentine.barshak@cogentembedded.com> >> --- >> Documentation/devicetree/bindings/ata/sata_rcar.txt | 16 ++++++++++++++++ >> arch/arm/boot/dts/r8a7779.dtsi | 2 +- >> drivers/ata/sata_rcar.c | 2 +- >> 3 files changed, 18 insertions(+), 2 deletions(-) >> create mode 100644 Documentation/devicetree/bindings/ata/sata_rcar.txt >> >> diff --git a/Documentation/devicetree/bindings/ata/sata_rcar.txt b/Documentation/devicetree/bindings/ata/sata_rcar.txt >> new file mode 100644 >> index 0000000..2465183 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/ata/sata_rcar.txt >> @@ -0,0 +1,16 @@ >> +* Renesas R-Car SATA >> + >> +Required properties: >> +- compatible : must be "renesas,sata-r8a7779" > > s/must be/should include/ > OK, thanks. >> +- reg : address range of the SATA registers. > > Also their size... Do you want me to rephrase it to "base address and size of the SATA registers"? Looks like a lot of other bindings examples available in Documentation/devicetree/bindings describe regs property as the "address range" which implies the address and size of the registers. > >> +- interrupt-parent : interrupt parent controller phandle > > This isn't required as such, as the interrupt-controller can be > inherited from the parent. As it's a standard auxiliary property I don't > think it needs to be documented here. OK, will drop it, thanks. > >> +- interrupts : must consist of one interrupt specifier. > > Is there only one interrupt generated by the device? Yes, there's just one IRQ. > >> + >> +Example: >> + >> +sata: sata@fc600000 { >> + compatible = "renesas,sata-r8a7779"; >> + reg = <0xfc600000 0x2000>; >> + interrupt-parent = <&gic>; >> + interrupts = <0 100 0x4>; >> +}; >> diff --git a/arch/arm/boot/dts/r8a7779.dtsi b/arch/arm/boot/dts/r8a7779.dtsi >> index da61d27..b1747f5 100644 >> --- a/arch/arm/boot/dts/r8a7779.dtsi >> +++ b/arch/arm/boot/dts/r8a7779.dtsi >> @@ -201,7 +201,7 @@ >> }; >> >> sata: sata@fc600000 { >> - compatible = "renesas,rcar-sata"; >> + compatible = "renesas,sata-r8a7779"; >> reg = <0xfc600000 0x2000>; >> interrupt-parent = <&gic>; >> interrupts = <0 100 0x4>; >> diff --git a/drivers/ata/sata_rcar.c b/drivers/ata/sata_rcar.c >> index c2d95e9..c4cd738 100644 >> --- a/drivers/ata/sata_rcar.c >> +++ b/drivers/ata/sata_rcar.c >> @@ -893,7 +893,7 @@ static const struct dev_pm_ops sata_rcar_pm_ops = { >> #endif >> >> static struct of_device_id sata_rcar_match[] = { >> - { .compatible = "renesas,rcar-sata", }, >> + { .compatible = "renesas,sata-r8a7779", }, >> {}, >> }; > > Why is an existing compatible string being removed? > > Please do not change existing strings, it will break any dts files that > are already in use. It won't break the dts files, since I'm changing the only dtsi file that uses the SATA node as well. It will only break binary compatibility with the older dtb files. > > While the new compatible string may be preferred, the old string should > be kept for compatibility. OK, I can keep the old string if it's needed. > > Thanks, > Mark. > Thanks, Val. -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Oct 14, 2013 at 09:19:37PM +0400, Valentine wrote: > Hi Mark, > thanks for looking at this. > > On 10/14/2013 08:13 PM, Mark Rutland wrote: > >On Mon, Oct 14, 2013 at 04:42:33PM +0100, Valentine Barshak wrote: > >>This converts the R-Car SATA DT compatibility string to > >>the <unit>-<soc> format which is the preferred one for > >>all SH-Mobile devices. The DT bindings are documented. > >> > >>Signed-off-by: Valentine Barshak <valentine.barshak@cogentembedded.com> > >>--- > >> Documentation/devicetree/bindings/ata/sata_rcar.txt | 16 ++++++++++++++++ > >> arch/arm/boot/dts/r8a7779.dtsi | 2 +- > >> drivers/ata/sata_rcar.c | 2 +- > >> 3 files changed, 18 insertions(+), 2 deletions(-) > >> create mode 100644 Documentation/devicetree/bindings/ata/sata_rcar.txt > >> > >>diff --git a/Documentation/devicetree/bindings/ata/sata_rcar.txt b/Documentation/devicetree/bindings/ata/sata_rcar.txt > >>new file mode 100644 > >>index 0000000..2465183 > >>--- /dev/null > >>+++ b/Documentation/devicetree/bindings/ata/sata_rcar.txt > >>@@ -0,0 +1,16 @@ > >>+* Renesas R-Car SATA > >>+ > >>+Required properties: > >>+- compatible : must be "renesas,sata-r8a7779" > > > >s/must be/should include/ > > > > OK, thanks. > > >>+- reg : address range of the SATA registers. > > > >Also their size... > > Do you want me to rephrase it to "base address and size of the SATA > registers"? > > Looks like a lot of other bindings examples available in > Documentation/devicetree/bindings describe regs property > as the "address range" which implies the address and size > of the registers. > > > > >>+- interrupt-parent : interrupt parent controller phandle > > > >This isn't required as such, as the interrupt-controller can be > >inherited from the parent. As it's a standard auxiliary property I don't > >think it needs to be documented here. > > OK, will drop it, thanks. > > > > >>+- interrupts : must consist of one interrupt specifier. > > > >Is there only one interrupt generated by the device? > > Yes, there's just one IRQ. > > > > >>+ > >>+Example: > >>+ > >>+sata: sata@fc600000 { > >>+ compatible = "renesas,sata-r8a7779"; > >>+ reg = <0xfc600000 0x2000>; > >>+ interrupt-parent = <&gic>; > >>+ interrupts = <0 100 0x4>; > >>+}; > >>diff --git a/arch/arm/boot/dts/r8a7779.dtsi b/arch/arm/boot/dts/r8a7779.dtsi > >>index da61d27..b1747f5 100644 > >>--- a/arch/arm/boot/dts/r8a7779.dtsi > >>+++ b/arch/arm/boot/dts/r8a7779.dtsi > >>@@ -201,7 +201,7 @@ > >> }; > >> > >> sata: sata@fc600000 { > >>- compatible = "renesas,rcar-sata"; > >>+ compatible = "renesas,sata-r8a7779"; > >> reg = <0xfc600000 0x2000>; > >> interrupt-parent = <&gic>; > >> interrupts = <0 100 0x4>; > >>diff --git a/drivers/ata/sata_rcar.c b/drivers/ata/sata_rcar.c > >>index c2d95e9..c4cd738 100644 > >>--- a/drivers/ata/sata_rcar.c > >>+++ b/drivers/ata/sata_rcar.c > >>@@ -893,7 +893,7 @@ static const struct dev_pm_ops sata_rcar_pm_ops = { > >> #endif > >> > >> static struct of_device_id sata_rcar_match[] = { > >>- { .compatible = "renesas,rcar-sata", }, > >>+ { .compatible = "renesas,sata-r8a7779", }, > >> {}, > >> }; > > > >Why is an existing compatible string being removed? > > > >Please do not change existing strings, it will break any dts files that > >are already in use. > > It won't break the dts files, since I'm changing the only dtsi file > that uses the SATA node as well. It will only break binary > compatibility with the older dtb files. I believe that compatibility with older dtb files is the key issue. > >While the new compatible string may be preferred, the old string should > >be kept for compatibility. > > OK, I can keep the old string if it's needed. FWIW I am doubtful that it is much of an issue in practice. However, just to be safe, I think it would be bes tto leave renesas,rcar-sata. -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Oct 14, 2013 at 06:19:37PM +0100, Valentine wrote: > Hi Mark, > thanks for looking at this. > > On 10/14/2013 08:13 PM, Mark Rutland wrote: > > On Mon, Oct 14, 2013 at 04:42:33PM +0100, Valentine Barshak wrote: > >> This converts the R-Car SATA DT compatibility string to > >> the <unit>-<soc> format which is the preferred one for > >> all SH-Mobile devices. The DT bindings are documented. > >> > >> Signed-off-by: Valentine Barshak <valentine.barshak@cogentembedded.com> > >> --- > >> Documentation/devicetree/bindings/ata/sata_rcar.txt | 16 ++++++++++++++++ > >> arch/arm/boot/dts/r8a7779.dtsi | 2 +- > >> drivers/ata/sata_rcar.c | 2 +- > >> 3 files changed, 18 insertions(+), 2 deletions(-) > >> create mode 100644 Documentation/devicetree/bindings/ata/sata_rcar.txt > >> > >> diff --git a/Documentation/devicetree/bindings/ata/sata_rcar.txt b/Documentation/devicetree/bindings/ata/sata_rcar.txt > >> new file mode 100644 > >> index 0000000..2465183 > >> --- /dev/null > >> +++ b/Documentation/devicetree/bindings/ata/sata_rcar.txt > >> @@ -0,0 +1,16 @@ > >> +* Renesas R-Car SATA > >> + > >> +Required properties: > >> +- compatible : must be "renesas,sata-r8a7779" > > > > s/must be/should include/ > > > > OK, thanks. Cheers. > > >> +- reg : address range of the SATA registers. > > > > Also their size... > > Do you want me to rephrase it to "base address and size of the SATA > registers"? > > Looks like a lot of other bindings examples available in > Documentation/devicetree/bindings describe regs property > as the "address range" which implies the address and size > of the registers. This is unfortunately one area where there isn't any general consensus on the terminology. I think "base address and size ..." is sufficient -- it maps well to the set of address-cells and size-cells. For reg values describing MMIO register blocks, ePAPR says "the offsets and lengths of the memory-mapped IO register blocks", and there's similar wording in other bindings. > > > > >> +- interrupt-parent : interrupt parent controller phandle > > > > This isn't required as such, as the interrupt-controller can be > > inherited from the parent. As it's a standard auxiliary property I don't > > think it needs to be documented here. > > OK, will drop it, thanks. Ok. > > > > >> +- interrupts : must consist of one interrupt specifier. > > > > Is there only one interrupt generated by the device? > > Yes, there's just one IRQ. Ok. > > > > >> + > >> +Example: > >> + > >> +sata: sata@fc600000 { > >> + compatible = "renesas,sata-r8a7779"; > >> + reg = <0xfc600000 0x2000>; > >> + interrupt-parent = <&gic>; > >> + interrupts = <0 100 0x4>; > >> +}; > >> diff --git a/arch/arm/boot/dts/r8a7779.dtsi b/arch/arm/boot/dts/r8a7779.dtsi > >> index da61d27..b1747f5 100644 > >> --- a/arch/arm/boot/dts/r8a7779.dtsi > >> +++ b/arch/arm/boot/dts/r8a7779.dtsi > >> @@ -201,7 +201,7 @@ > >> }; > >> > >> sata: sata@fc600000 { > >> - compatible = "renesas,rcar-sata"; > >> + compatible = "renesas,sata-r8a7779"; > >> reg = <0xfc600000 0x2000>; > >> interrupt-parent = <&gic>; > >> interrupts = <0 100 0x4>; > >> diff --git a/drivers/ata/sata_rcar.c b/drivers/ata/sata_rcar.c > >> index c2d95e9..c4cd738 100644 > >> --- a/drivers/ata/sata_rcar.c > >> +++ b/drivers/ata/sata_rcar.c > >> @@ -893,7 +893,7 @@ static const struct dev_pm_ops sata_rcar_pm_ops = { > >> #endif > >> > >> static struct of_device_id sata_rcar_match[] = { > >> - { .compatible = "renesas,rcar-sata", }, > >> + { .compatible = "renesas,sata-r8a7779", }, > >> {}, > >> }; > > > > Why is an existing compatible string being removed? > > > > Please do not change existing strings, it will break any dts files that > > are already in use. > > It won't break the dts files, since I'm changing the only dtsi file that > uses the SATA node as well. It will only break binary compatibility with > the older dtb files. Why should an existing dtb suddenly stop working? What if someone has an out-of-tree dts? > > > > > While the new compatible string may be preferred, the old string should > > be kept for compatibility. > > OK, I can keep the old string if it's needed. Please do. Thanks, Mark. -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/Documentation/devicetree/bindings/ata/sata_rcar.txt b/Documentation/devicetree/bindings/ata/sata_rcar.txt new file mode 100644 index 0000000..2465183 --- /dev/null +++ b/Documentation/devicetree/bindings/ata/sata_rcar.txt @@ -0,0 +1,16 @@ +* Renesas R-Car SATA + +Required properties: +- compatible : must be "renesas,sata-r8a7779" +- reg : address range of the SATA registers. +- interrupt-parent : interrupt parent controller phandle +- interrupts : must consist of one interrupt specifier. + +Example: + +sata: sata@fc600000 { + compatible = "renesas,sata-r8a7779"; + reg = <0xfc600000 0x2000>; + interrupt-parent = <&gic>; + interrupts = <0 100 0x4>; +}; diff --git a/arch/arm/boot/dts/r8a7779.dtsi b/arch/arm/boot/dts/r8a7779.dtsi index da61d27..b1747f5 100644 --- a/arch/arm/boot/dts/r8a7779.dtsi +++ b/arch/arm/boot/dts/r8a7779.dtsi @@ -201,7 +201,7 @@ }; sata: sata@fc600000 { - compatible = "renesas,rcar-sata"; + compatible = "renesas,sata-r8a7779"; reg = <0xfc600000 0x2000>; interrupt-parent = <&gic>; interrupts = <0 100 0x4>; diff --git a/drivers/ata/sata_rcar.c b/drivers/ata/sata_rcar.c index c2d95e9..c4cd738 100644 --- a/drivers/ata/sata_rcar.c +++ b/drivers/ata/sata_rcar.c @@ -893,7 +893,7 @@ static const struct dev_pm_ops sata_rcar_pm_ops = { #endif static struct of_device_id sata_rcar_match[] = { - { .compatible = "renesas,rcar-sata", }, + { .compatible = "renesas,sata-r8a7779", }, {}, }; MODULE_DEVICE_TABLE(of, sata_rcar_match);
This converts the R-Car SATA DT compatibility string to the <unit>-<soc> format which is the preferred one for all SH-Mobile devices. The DT bindings are documented. Signed-off-by: Valentine Barshak <valentine.barshak@cogentembedded.com> --- Documentation/devicetree/bindings/ata/sata_rcar.txt | 16 ++++++++++++++++ arch/arm/boot/dts/r8a7779.dtsi | 2 +- drivers/ata/sata_rcar.c | 2 +- 3 files changed, 18 insertions(+), 2 deletions(-) create mode 100644 Documentation/devicetree/bindings/ata/sata_rcar.txt