| Message ID | 1421274213-3544-4-git-send-email-rjui@broadcom.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hello, On Wed, Jan 14, 2015 at 02:23:33PM -0800, Ray Jui wrote: > Add I2C device nodes and its properties in bcm-cygnus.dtsi but keep > them disabled there. Individual I2C devices can be enabled in board > specific dts file when I2C slave devices are enabled in the future s/$/./ > > Signed-off-by: Ray Jui <rjui@broadcom.com> > Reviewed-by: Scott Branden <sbranden@broadcom.com> > --- > arch/arm/boot/dts/bcm-cygnus.dtsi | 20 ++++++++++++++++++++ > 1 file changed, 20 insertions(+) > > diff --git a/arch/arm/boot/dts/bcm-cygnus.dtsi b/arch/arm/boot/dts/bcm-cygnus.dtsi > index 5126f9e..f7d6c1d 100644 > --- a/arch/arm/boot/dts/bcm-cygnus.dtsi > +++ b/arch/arm/boot/dts/bcm-cygnus.dtsi > @@ -70,6 +70,26 @@ > }; > }; > > + i2c0: i2c@18008000 { > + compatible = "brcm,iproc-i2c"; in patch 2 you wrote the driver is for a family of SoCs, right? Then I'd make this: compatible = "brcm,$mysoc-iproc-i2c", "brcm,iproc-i2c"; (or maybe s/$mysoc-iproc-i2c/$mysoc-i2c/). Best regards Uwe
On 1/15/2015 12:44 AM, Uwe Kleine-König wrote: > Hello, > > On Wed, Jan 14, 2015 at 02:23:33PM -0800, Ray Jui wrote: >> Add I2C device nodes and its properties in bcm-cygnus.dtsi but keep >> them disabled there. Individual I2C devices can be enabled in board >> specific dts file when I2C slave devices are enabled in the future > s/$/./ > >> >> Signed-off-by: Ray Jui <rjui@broadcom.com> >> Reviewed-by: Scott Branden <sbranden@broadcom.com> >> --- >> arch/arm/boot/dts/bcm-cygnus.dtsi | 20 ++++++++++++++++++++ >> 1 file changed, 20 insertions(+) >> >> diff --git a/arch/arm/boot/dts/bcm-cygnus.dtsi b/arch/arm/boot/dts/bcm-cygnus.dtsi >> index 5126f9e..f7d6c1d 100644 >> --- a/arch/arm/boot/dts/bcm-cygnus.dtsi >> +++ b/arch/arm/boot/dts/bcm-cygnus.dtsi >> @@ -70,6 +70,26 @@ >> }; >> }; >> >> + i2c0: i2c@18008000 { >> + compatible = "brcm,iproc-i2c"; > in patch 2 you wrote the driver is for a family of SoCs, right? Then I'd > make this: > > compatible = "brcm,$mysoc-iproc-i2c", "brcm,iproc-i2c"; > Sorry could you please help to explain the intention here? Note the iProc I2C IP can be found in various iProc family of SoCs, but to my best knowledge, there hasn't been any changes of the IP in any of those SoCs. Is the compatible ID "brcm,$mysoc-iproc-i2c" only to clarify that it's for a specific SoC? If so, what should the compatible ID array look like? Should it be changed to the following? static const struct of_device_id bcm_iproc_i2c_of_match[] = { { .compatible = "brcm,iproc-i2c" }, { .compatible = "brcm,$mysoc-iproc-i2c" }, {}, }; > (or maybe s/$mysoc-iproc-i2c/$mysoc-i2c/). > > Best regards > Uwe >
Hello, On Fri, Jan 16, 2015 at 11:24:09AM -0800, Ray Jui wrote: > >> + i2c0: i2c@18008000 { > >> + compatible = "brcm,iproc-i2c"; > > in patch 2 you wrote the driver is for a family of SoCs, right? Then I'd > > make this: > > > > compatible = "brcm,$mysoc-iproc-i2c", "brcm,iproc-i2c"; > > > Sorry could you please help to explain the intention here? Note the > iProc I2C IP can be found in various iProc family of SoCs, but to my > best knowledge, there hasn't been any changes of the IP in any of those > SoCs. This is just for making the device tree stable in the future. Consider your gentle hardware engineers "fix" a small issue for the next generation iproc SoC "pony" that needs an incompatible software change. Then you can fix the driver without updating the device trees by switching to the SoC specific compatible string for "pony". And in case the hardware engineers didn't tell you that there is a change and the need for the software change is only detected when the machines are already shipped, you're happy if you can fix your kernel without needing to change the bootloader that provides the dtb. So start already today to add the (for now unused) compatible string. > Is the compatible ID "brcm,$mysoc-iproc-i2c" only to clarify that it's > for a specific SoC? If so, what should the compatible ID array look > like? Should it be changed to the following? > > static const struct of_device_id bcm_iproc_i2c_of_match[] = { > { .compatible = "brcm,iproc-i2c" }, > { .compatible = "brcm,$mysoc-iproc-i2c" }, > {}, > }; No, there is no need, see above. If something is still unclear, don't hesitate to ask. Best regards Uwe
On 1/16/2015 11:48 AM, Uwe Kleine-König wrote: > Hello, > > On Fri, Jan 16, 2015 at 11:24:09AM -0800, Ray Jui wrote: >>>> + i2c0: i2c@18008000 { >>>> + compatible = "brcm,iproc-i2c"; >>> in patch 2 you wrote the driver is for a family of SoCs, right? Then I'd >>> make this: >>> >>> compatible = "brcm,$mysoc-iproc-i2c", "brcm,iproc-i2c"; >>> >> Sorry could you please help to explain the intention here? Note the >> iProc I2C IP can be found in various iProc family of SoCs, but to my >> best knowledge, there hasn't been any changes of the IP in any of those >> SoCs. > This is just for making the device tree stable in the future. Consider > your gentle hardware engineers "fix" a small issue for the next > generation iproc SoC "pony" that needs an incompatible software change. > > Then you can fix the driver without updating the device trees by > switching to the SoC specific compatible string for "pony". And in case > the hardware engineers didn't tell you that there is a change and the > need for the software change is only detected when the machines are > already shipped, you're happy if you can fix your kernel without needing > to change the bootloader that provides the dtb. > So start already today to add the (for now unused) compatible string. > >> Is the compatible ID "brcm,$mysoc-iproc-i2c" only to clarify that it's >> for a specific SoC? If so, what should the compatible ID array look >> like? Should it be changed to the following? >> >> static const struct of_device_id bcm_iproc_i2c_of_match[] = { >> { .compatible = "brcm,iproc-i2c" }, >> { .compatible = "brcm,$mysoc-iproc-i2c" }, >> {}, >> }; > No, there is no need, see above. > > If something is still unclear, don't hesitate to ask. > > Best regards > Uwe > Okay got it. Thanks for the explanation!
diff --git a/arch/arm/boot/dts/bcm-cygnus.dtsi b/arch/arm/boot/dts/bcm-cygnus.dtsi index 5126f9e..f7d6c1d 100644 --- a/arch/arm/boot/dts/bcm-cygnus.dtsi +++ b/arch/arm/boot/dts/bcm-cygnus.dtsi @@ -70,6 +70,26 @@ }; }; + i2c0: i2c@18008000 { + compatible = "brcm,iproc-i2c"; + reg = <0x18008000 0x100>; + #address-cells = <1>; + #size-cells = <0>; + interrupts = <GIC_SPI 85 IRQ_TYPE_NONE>; + clock-frequency = <100000>; + status = "disabled"; + }; + + i2c1: i2c@1800b000 { + compatible = "brcm,iproc-i2c"; + reg = <0x1800b000 0x100>; + #address-cells = <1>; + #size-cells = <0>; + interrupts = <GIC_SPI 86 IRQ_TYPE_NONE>; + clock-frequency = <100000>; + status = "disabled"; + }; + uart0: serial@18020000 { compatible = "snps,dw-apb-uart"; reg = <0x18020000 0x100>;