| Message ID | 1413538026-15739-1-git-send-email-valentin.longchamp@keymile.com |
|---|---|
| State | Not Applicable |
| Headers | show |
On Fri, 2014-10-17 at 11:27 +0200, Valentin Longchamp wrote: > With "fsl-i2c" compatibility the i2c frequency is not set > correctly, because it sets no prescaler. According to the AN2919 from > Freescale and the QorIQ (P2041) documentation, the source clock is 1/2 > the platform clock. This implies that a prescaler of 2 must be used. > > This changes the compatibility of the qoriq-i2c .dtsi files to pick the > mpc8543, which uses the same driver but sets the correct prescaler. > > Signed-off-by: Rainer Boschung <rainer.boschung@keymile.com> > Signed-off-by: Valentin Longchamp <valentin.longchamp@keymile.com> > --- > > arch/powerpc/boot/dts/fsl/qoriq-i2c-0.dtsi | 4 ++-- > arch/powerpc/boot/dts/fsl/qoriq-i2c-1.dtsi | 4 ++-- > 2 files changed, 4 insertions(+), 4 deletions(-) > > diff --git a/arch/powerpc/boot/dts/fsl/qoriq-i2c-0.dtsi b/arch/powerpc/boot/dts/fsl/qoriq-i2c-0.dtsi > index 5f9bf7d..aa6c366 100644 > --- a/arch/powerpc/boot/dts/fsl/qoriq-i2c-0.dtsi > +++ b/arch/powerpc/boot/dts/fsl/qoriq-i2c-0.dtsi > @@ -36,7 +36,7 @@ i2c@118000 { > #address-cells = <1>; > #size-cells = <0>; > cell-index = <0>; > - compatible = "fsl-i2c"; > + compatible = "fsl,mpc8543-i2c", "fsl-i2c"; > reg = <0x118000 0x100>; > interrupts = <38 2 0 0>; > dfsrr; > @@ -46,7 +46,7 @@ i2c@118100 { > #address-cells = <1>; > #size-cells = <0>; > cell-index = <1>; > - compatible = "fsl-i2c"; > + compatible = "fsl,mpc8543-i2c", "fsl-i2c"; > reg = <0x118100 0x100>; > interrupts = <38 2 0 0>; > dfsrr; Are all chips that use this dtsi 100% compatible with mpc8543's i2c, or just in ways the Linux driver cares about? What about fsl,mpc8544-i2c, which has additional special handling in the driver, but is only used in socrates.dts (not mpc8544ds.dts)? What about pq3-i2c-*.dtsi? -Scott -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 10/29/2014 12:08 AM, Scott Wood wrote: > On Fri, 2014-10-17 at 11:27 +0200, Valentin Longchamp wrote: >> With "fsl-i2c" compatibility the i2c frequency is not set >> correctly, because it sets no prescaler. According to the AN2919 from >> Freescale and the QorIQ (P2041) documentation, the source clock is 1/2 >> the platform clock. This implies that a prescaler of 2 must be used. >> >> This changes the compatibility of the qoriq-i2c .dtsi files to pick the >> mpc8543, which uses the same driver but sets the correct prescaler. >> >> Signed-off-by: Rainer Boschung <rainer.boschung@keymile.com> >> Signed-off-by: Valentin Longchamp <valentin.longchamp@keymile.com> >> --- >> >> arch/powerpc/boot/dts/fsl/qoriq-i2c-0.dtsi | 4 ++-- >> arch/powerpc/boot/dts/fsl/qoriq-i2c-1.dtsi | 4 ++-- >> 2 files changed, 4 insertions(+), 4 deletions(-) >> >> diff --git a/arch/powerpc/boot/dts/fsl/qoriq-i2c-0.dtsi b/arch/powerpc/boot/dts/fsl/qoriq-i2c-0.dtsi >> index 5f9bf7d..aa6c366 100644 >> --- a/arch/powerpc/boot/dts/fsl/qoriq-i2c-0.dtsi >> +++ b/arch/powerpc/boot/dts/fsl/qoriq-i2c-0.dtsi >> @@ -36,7 +36,7 @@ i2c@118000 { >> #address-cells = <1>; >> #size-cells = <0>; >> cell-index = <0>; >> - compatible = "fsl-i2c"; >> + compatible = "fsl,mpc8543-i2c", "fsl-i2c"; >> reg = <0x118000 0x100>; >> interrupts = <38 2 0 0>; >> dfsrr; >> @@ -46,7 +46,7 @@ i2c@118100 { >> #address-cells = <1>; >> #size-cells = <0>; >> cell-index = <1>; >> - compatible = "fsl-i2c"; >> + compatible = "fsl,mpc8543-i2c", "fsl-i2c"; >> reg = <0x118100 0x100>; >> interrupts = <38 2 0 0>; >> dfsrr; > > Are all chips that use this dtsi 100% compatible with mpc8543's i2c, or > just in ways the Linux driver cares about? I have just looked briefly at the mpc8548 RM (covers mpc8543) and its i2c controller looks the same as the qoriq's. I cannot however state if they are 100% compatible. If we wanted to be on the safe side and strict (since we are not sure that the hardware is 100% compatible), we maybe should add a fsl,qoriq-i2c compatible to the driver that does the same as mpc8543-i2c. > > What about fsl,mpc8544-i2c, which has additional special handling in the > driver, but is only used in socrates.dts (not mpc8544ds.dts)? From the mpc8544 RM, this controller looks the same as the above 2, except for the prescaler from the driver which is set to 3. As to why it is only used in the socrates.dts, I cannot comment about it. The prescaler is confirmed to be 3 by default by the Table 3 of the AN-219 for the mpc8544. > > What about pq3-i2c-*.dtsi? > This is also interesting: from the AN-219 Table 4, some pq3 have a 2:1 (mcpc8536/43/45/47/48/67/68/72, plus p2020) prescaler where some don't (mpc8533/44, where it can be 3:1 -default- or 2:1). However pq3-i2c-*.dtsi defines no prescaler. Now if I look at what files include these pq3-i2c-*.dtsi, I see some that are in the the 2:1 list: arch/powerpc/boot/dts/fsl/mpc8536si-post.dtsi arch/powerpc/boot/dts/fsl/mpc8548si-post.dtsi arch/powerpc/boot/dts/fsl/mpc8568si-post.dtsi arch/powerpc/boot/dts/fsl/mpc8572si-post.dtsi arch/powerpc/boot/dts/fsl/p2020si-post.dtsi I don't have any hardware to do some tests with these, but from my measurements on our qoriq based system (P2041 SoC) I think that the generated I2C clocks for the above SoC currently are not correct because of the ignored prescaler. Valentin -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 2014-10-29 at 09:59 +0100, Valentin Longchamp wrote: > On 10/29/2014 12:08 AM, Scott Wood wrote: > > On Fri, 2014-10-17 at 11:27 +0200, Valentin Longchamp wrote: > >> With "fsl-i2c" compatibility the i2c frequency is not set > >> correctly, because it sets no prescaler. According to the AN2919 from > >> Freescale and the QorIQ (P2041) documentation, the source clock is 1/2 > >> the platform clock. This implies that a prescaler of 2 must be used. > >> > >> This changes the compatibility of the qoriq-i2c .dtsi files to pick the > >> mpc8543, which uses the same driver but sets the correct prescaler. > >> > >> Signed-off-by: Rainer Boschung <rainer.boschung@keymile.com> > >> Signed-off-by: Valentin Longchamp <valentin.longchamp@keymile.com> > >> --- > >> > >> arch/powerpc/boot/dts/fsl/qoriq-i2c-0.dtsi | 4 ++-- > >> arch/powerpc/boot/dts/fsl/qoriq-i2c-1.dtsi | 4 ++-- > >> 2 files changed, 4 insertions(+), 4 deletions(-) > >> > >> diff --git a/arch/powerpc/boot/dts/fsl/qoriq-i2c-0.dtsi b/arch/powerpc/boot/dts/fsl/qoriq-i2c-0.dtsi > >> index 5f9bf7d..aa6c366 100644 > >> --- a/arch/powerpc/boot/dts/fsl/qoriq-i2c-0.dtsi > >> +++ b/arch/powerpc/boot/dts/fsl/qoriq-i2c-0.dtsi > >> @@ -36,7 +36,7 @@ i2c@118000 { > >> #address-cells = <1>; > >> #size-cells = <0>; > >> cell-index = <0>; > >> - compatible = "fsl-i2c"; > >> + compatible = "fsl,mpc8543-i2c", "fsl-i2c"; > >> reg = <0x118000 0x100>; > >> interrupts = <38 2 0 0>; > >> dfsrr; > >> @@ -46,7 +46,7 @@ i2c@118100 { > >> #address-cells = <1>; > >> #size-cells = <0>; > >> cell-index = <1>; > >> - compatible = "fsl-i2c"; > >> + compatible = "fsl,mpc8543-i2c", "fsl-i2c"; > >> reg = <0x118100 0x100>; > >> interrupts = <38 2 0 0>; > >> dfsrr; > > > > Are all chips that use this dtsi 100% compatible with mpc8543's i2c, or > > just in ways the Linux driver cares about? > > I have just looked briefly at the mpc8548 RM (covers mpc8543) and its i2c > controller looks the same as the qoriq's. I cannot however state if they are > 100% compatible. > > If we wanted to be on the safe side and strict (since we are not sure that the > hardware is 100% compatible), we maybe should add a fsl,qoriq-i2c compatible to > the driver that does the same as mpc8543-i2c. If we're going to change the device tree I'd rather just add a property to say what the prescaler is. -Scott -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> If we wanted to be on the safe side and strict (since we are not sure that the > hardware is 100% compatible), we maybe should add a fsl,qoriq-i2c compatible to > the driver that does the same as mpc8543-i2c. Or you leave the driver as is and use both compatibles: compatible = "fsl,qoriq-i2c", "fsl,mpc8543-i2c", "fsl-i2c"; ?
On 11/13/2014 01:34 AM, Wolfram Sang wrote: > >> If we wanted to be on the safe side and strict (since we are not sure that the >> hardware is 100% compatible), we maybe should add a fsl,qoriq-i2c compatible to >> the driver that does the same as mpc8543-i2c. > > Or you leave the driver as is and use both compatibles: > > compatible = "fsl,qoriq-i2c", "fsl,mpc8543-i2c", "fsl-i2c"; > > ? > I like Scott's proposition to add the prescaler in the device tree more. From the hardware description point of view, it makes more sense: the devices are all just fsl-i2c, with a different prescaler. I just quote it below as a reminder. > > If we're going to change the device tree I'd rather just add a property > to say what the prescaler is. We would however, leave the boards' device trees that use things like "fsl,mpc8543-i2c" as is and introduce the prescaler for the others requiring it. Now the drawback is that the driver would require a change, to parse this prescaler new prescaler property. Would this be OK from your point of view Wolfram ? If yes, I will send the patches for it. Valentin -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> > > > If we're going to change the device tree I'd rather just add a property > > to say what the prescaler is. > > We would however, leave the boards' device trees that use things like > "fsl,mpc8543-i2c" as is and introduce the prescaler for the others requiring it. > > > Now the drawback is that the driver would require a change, to parse this > prescaler new prescaler property. Would this be OK from your point of view > Wolfram ? If yes, I will send the patches for it. I don't think it is OK. I'd think it can be deduced from the compatible property. Let's ask the DT experts here. I'll follow their suggestion.
On Fri, 2014-11-14 at 09:28 +0100, Wolfram Sang wrote: > > > > > > If we're going to change the device tree I'd rather just add a property > > > to say what the prescaler is. > > > > We would however, leave the boards' device trees that use things like > > "fsl,mpc8543-i2c" as is and introduce the prescaler for the others requiring it. > > > > > > Now the drawback is that the driver would require a change, to parse this > > prescaler new prescaler property. Would this be OK from your point of view > > Wolfram ? If yes, I will send the patches for it. > > I don't think it is OK. Why? > I'd think it can be deduced from the compatible property. For almost all existing device trees it cannot be. If you want something that will work without changing device trees, you'll need to use SVR to identify the SoC. -Scott -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Nov 17, 2014 at 07:28:03PM -0600, Scott Wood wrote: > On Fri, 2014-11-14 at 09:28 +0100, Wolfram Sang wrote: > > > > > > > > If we're going to change the device tree I'd rather just add a property > > > > to say what the prescaler is. > > > > > > We would however, leave the boards' device trees that use things like > > > "fsl,mpc8543-i2c" as is and introduce the prescaler for the others requiring it. > > > > > > > > > Now the drawback is that the driver would require a change, to parse this > > > prescaler new prescaler property. Would this be OK from your point of view > > > Wolfram ? If yes, I will send the patches for it. > > > > I don't think it is OK. > > Why? Because I thought it could be deduced. Then, a seperate property would not be OK. > > I'd think it can be deduced from the compatible property. > > For almost all existing device trees it cannot be. Pity :( If we do introduce a new property, it should probably be "clock-div". Grepping through binding documentation, that seems accepted. We should ask DT maintainers, too, to be safe. > If you want something that will work without changing device trees, > you'll need to use SVR to identify the SoC. The driver is doing that already, see mpc_i2c_get_sec_cfg_8xxx(). Dunno if it makes sense to add to it for consistency reasons?
On Tue, 2014-11-25 at 19:13 +0100, Wolfram Sang wrote: > On Mon, Nov 17, 2014 at 07:28:03PM -0600, Scott Wood wrote: > > On Fri, 2014-11-14 at 09:28 +0100, Wolfram Sang wrote: > > > > > > > > > > If we're going to change the device tree I'd rather just add a property > > > > > to say what the prescaler is. > > > > > > > > We would however, leave the boards' device trees that use things like > > > > "fsl,mpc8543-i2c" as is and introduce the prescaler for the others requiring it. > > > > > > > > > > > > Now the drawback is that the driver would require a change, to parse this > > > > prescaler new prescaler property. Would this be OK from your point of view > > > > Wolfram ? If yes, I will send the patches for it. > > > > > > I don't think it is OK. > > > > Why? > > Because I thought it could be deduced. Then, a seperate property would > not be OK. > > > > I'd think it can be deduced from the compatible property. > > > > For almost all existing device trees it cannot be. > > Pity :( If we do introduce a new property, it should probably be > "clock-div". Grepping through binding documentation, that seems > accepted. We should ask DT maintainers, too, to be safe. > > > If you want something that will work without changing device trees, > > you'll need to use SVR to identify the SoC. > > The driver is doing that already, see mpc_i2c_get_sec_cfg_8xxx(). Dunno > if it makes sense to add to it for consistency reasons? That's not SVR, but sure. Better to avoid messing with existing device trees. -Scott -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
I saw that this patch was marked as not applicable, but on most qoriq devices the pre-scaler is 2 especially for p2020/p2010 devices arch/powerpc/boot/dts/fsl/p2020si-post.dtsi from the P2020 RM: p. 477 "Frequency divider ratio. Used to prescale the clock for bit rate selection. The serial bit clock frequency of SCL is equal to one half the platform ( CCB ) clock divided by the designated divider ." This means that the current dts for these devices are providing false clock settings. I have a p2020 board and can take some scope measurements next week to prove this. I think something should be modified to address this. On Tue, Nov 25, 2014 at 5:41 PM, Scott Wood <scottwood@freescale.com> wrote: > On Tue, 2014-11-25 at 19:13 +0100, Wolfram Sang wrote: >> On Mon, Nov 17, 2014 at 07:28:03PM -0600, Scott Wood wrote: >> > On Fri, 2014-11-14 at 09:28 +0100, Wolfram Sang wrote: >> > > > > >> > > > > If we're going to change the device tree I'd rather just add a property >> > > > > to say what the prescaler is. >> > > > >> > > > We would however, leave the boards' device trees that use things like >> > > > "fsl,mpc8543-i2c" as is and introduce the prescaler for the others requiring it. >> > > > >> > > > >> > > > Now the drawback is that the driver would require a change, to parse this >> > > > prescaler new prescaler property. Would this be OK from your point of view >> > > > Wolfram ? If yes, I will send the patches for it. >> > > >> > > I don't think it is OK. >> > >> > Why? >> >> Because I thought it could be deduced. Then, a seperate property would >> not be OK. >> >> > > I'd think it can be deduced from the compatible property. >> > >> > For almost all existing device trees it cannot be. >> >> Pity :( If we do introduce a new property, it should probably be >> "clock-div". Grepping through binding documentation, that seems >> accepted. We should ask DT maintainers, too, to be safe. >> >> > If you want something that will work without changing device trees, >> > you'll need to use SVR to identify the SoC. >> >> The driver is doing that already, see mpc_i2c_get_sec_cfg_8xxx(). Dunno >> if it makes sense to add to it for consistency reasons? > > That's not SVR, but sure. Better to avoid messing with existing device > trees. > > -Scott > > > -- > To unsubscribe from this list: send the line "unsubscribe linux-i2c" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
> I saw that this patch was marked as not applicable, but on most qoriq > devices the pre-scaler is 2 especially for p2020/p2010 devices > arch/powerpc/boot/dts/fsl/p2020si-post.dtsi Just for completeness: "Not applicable" given from patchwork of the i2c subsystem means this patch is not for the i2c subsystem. In this case, it is for powerpc because it was modifying powerpc dts files only. That doesn't say anything about the patch itself.
Hi all, Picking up this issue again. On 11/26/2014 02:41 AM, Scott Wood wrote: > On Tue, 2014-11-25 at 19:13 +0100, Wolfram Sang wrote: >> On Mon, Nov 17, 2014 at 07:28:03PM -0600, Scott Wood wrote: >>> On Fri, 2014-11-14 at 09:28 +0100, Wolfram Sang wrote: >>>>>> >>>>>> If we're going to change the device tree I'd rather just add a property >>>>>> to say what the prescaler is. >>>>> >>>>> We would however, leave the boards' device trees that use things like >>>>> "fsl,mpc8543-i2c" as is and introduce the prescaler for the others requiring it. >>>>> >>>>> >>>>> Now the drawback is that the driver would require a change, to parse this >>>>> prescaler new prescaler property. Would this be OK from your point of view >>>>> Wolfram ? If yes, I will send the patches for it. >>>> >>>> I don't think it is OK. >>> >>> Why? >> >> Because I thought it could be deduced. Then, a seperate property would >> not be OK. >> >>>> I'd think it can be deduced from the compatible property. >>> >>> For almost all existing device trees it cannot be. >> >> Pity :( If we do introduce a new property, it should probably be >> "clock-div". Grepping through binding documentation, that seems >> accepted. We should ask DT maintainers, too, to be safe. >> >>> If you want something that will work without changing device trees, >>> you'll need to use SVR to identify the SoC. >> >> The driver is doing that already, see mpc_i2c_get_sec_cfg_8xxx(). Dunno >> if it makes sense to add to it for consistency reasons? > > That's not SVR, but sure. Better to avoid messing with existing device > trees. > What is then the agreement here ? Add a clock-div to the device trees ? Or do something similar to mpc_i2c_get_sec_cfg_8xxx() ? I think the clock-div property is better according to Freescale's AN 2919 section 3.1 Source clock. All the source clocks are fixed (with a clock-div of 2 in case of mpc8536/43/45/47/48/67/68/72, plus p2020) except for the mpc8533/44 where it can be 2 or 3, and that's what mpc_i2c_get_sec_cfg_8xxx() determines. So mpc_i2c_get_sec_cfg_8xxx() should remain the exception and the other prescaler values should be derived from an additional clock-div that must be added in the respective device trees (at least for the qoriq devices, because for instance mpc8543 already has the correct prescaler thanks to mpc_i2c_data_8543 from i2c-mpc.c). Valentin -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Wolfgang, Scott, On 12/11/2014 02:44 PM, Valentin Longchamp wrote: > Hi all, > > Picking up this issue again. > > On 11/26/2014 02:41 AM, Scott Wood wrote: >> On Tue, 2014-11-25 at 19:13 +0100, Wolfram Sang wrote: >>> On Mon, Nov 17, 2014 at 07:28:03PM -0600, Scott Wood wrote: >>>> On Fri, 2014-11-14 at 09:28 +0100, Wolfram Sang wrote: >>>>>>> >>>>>>> If we're going to change the device tree I'd rather just add a property >>>>>>> to say what the prescaler is. >>>>>> >>>>>> We would however, leave the boards' device trees that use things like >>>>>> "fsl,mpc8543-i2c" as is and introduce the prescaler for the others requiring it. >>>>>> >>>>>> >>>>>> Now the drawback is that the driver would require a change, to parse this >>>>>> prescaler new prescaler property. Would this be OK from your point of view >>>>>> Wolfram ? If yes, I will send the patches for it. >>>>> >>>>> I don't think it is OK. >>>> >>>> Why? >>> >>> Because I thought it could be deduced. Then, a seperate property would >>> not be OK. >>> >>>>> I'd think it can be deduced from the compatible property. >>>> >>>> For almost all existing device trees it cannot be. >>> >>> Pity :( If we do introduce a new property, it should probably be >>> "clock-div". Grepping through binding documentation, that seems >>> accepted. We should ask DT maintainers, too, to be safe. >>> >>>> If you want something that will work without changing device trees, >>>> you'll need to use SVR to identify the SoC. >>> >>> The driver is doing that already, see mpc_i2c_get_sec_cfg_8xxx(). Dunno >>> if it makes sense to add to it for consistency reasons? >> >> That's not SVR, but sure. Better to avoid messing with existing device >> trees. >> > > What is then the agreement here ? Add a clock-div to the device trees ? Or do > something similar to mpc_i2c_get_sec_cfg_8xxx() ? > > I think the clock-div property is better according to Freescale's AN 2919 > section 3.1 Source clock. All the source clocks are fixed (with a clock-div of 2 > in case of mpc8536/43/45/47/48/67/68/72, plus p2020) except for the mpc8533/44 > where it can be 2 or 3, and that's what mpc_i2c_get_sec_cfg_8xxx() determines. > > So mpc_i2c_get_sec_cfg_8xxx() should remain the exception and the other > prescaler values should be derived from an additional clock-div that must be > added in the respective device trees (at least for the qoriq devices, because > for instance mpc8543 already has the correct prescaler thanks to > mpc_i2c_data_8543 from i2c-mpc.c). > Do you have an opinion on the above ? Valentin -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Dec 23, 2014 at 02:23:01PM +0100, Valentin Longchamp wrote: > Wolfgang, Scott, Wolfram, please. > > What is then the agreement here ? Add a clock-div to the device trees ? Or do > > something similar to mpc_i2c_get_sec_cfg_8xxx() ? > > > > I think the clock-div property is better according to Freescale's AN 2919 > > section 3.1 Source clock. All the source clocks are fixed (with a clock-div of 2 > > in case of mpc8536/43/45/47/48/67/68/72, plus p2020) except for the mpc8533/44 > > where it can be 2 or 3, and that's what mpc_i2c_get_sec_cfg_8xxx() determines. > > > > So mpc_i2c_get_sec_cfg_8xxx() should remain the exception and the other > > prescaler values should be derived from an additional clock-div that must be > > added in the respective device trees (at least for the qoriq devices, because > > for instance mpc8543 already has the correct prescaler thanks to > > mpc_i2c_data_8543 from i2c-mpc.c). > > > > Do you have an opinion on the above ? I don't mind. I'll leave it to PowerPC experts to judge if a new binding is apropriate or reading SVR is the way to go. If it is going to be a new binding, then please look around before if there is already something similar around...
On Tue, 2014-12-23 at 14:49 +0100, Wolfram Sang wrote: > On Tue, Dec 23, 2014 at 02:23:01PM +0100, Valentin Longchamp wrote: > > Wolfgang, Scott, > > Wolfram, please. > > > > What is then the agreement here ? Add a clock-div to the device trees ? Or do > > > something similar to mpc_i2c_get_sec_cfg_8xxx() ? > > > > > > I think the clock-div property is better according to Freescale's AN 2919 > > > section 3.1 Source clock. All the source clocks are fixed (with a clock-div of 2 > > > in case of mpc8536/43/45/47/48/67/68/72, plus p2020) except for the mpc8533/44 > > > where it can be 2 or 3, and that's what mpc_i2c_get_sec_cfg_8xxx() determines. > > > > > > So mpc_i2c_get_sec_cfg_8xxx() should remain the exception and the other > > > prescaler values should be derived from an additional clock-div that must be > > > added in the respective device trees (at least for the qoriq devices, because > > > for instance mpc8543 already has the correct prescaler thanks to > > > mpc_i2c_data_8543 from i2c-mpc.c). > > > > > > > Do you have an opinion on the above ? > > I don't mind. I'll leave it to PowerPC experts to judge if a new binding > is apropriate or reading SVR is the way to go. If it is going to be a > new binding, then please look around before if there is already > something similar around... I'd rather use SVR so things work with existing device trees. -Scott -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" 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/arch/powerpc/boot/dts/fsl/qoriq-i2c-0.dtsi b/arch/powerpc/boot/dts/fsl/qoriq-i2c-0.dtsi index 5f9bf7d..aa6c366 100644 --- a/arch/powerpc/boot/dts/fsl/qoriq-i2c-0.dtsi +++ b/arch/powerpc/boot/dts/fsl/qoriq-i2c-0.dtsi @@ -36,7 +36,7 @@ i2c@118000 { #address-cells = <1>; #size-cells = <0>; cell-index = <0>; - compatible = "fsl-i2c"; + compatible = "fsl,mpc8543-i2c", "fsl-i2c"; reg = <0x118000 0x100>; interrupts = <38 2 0 0>; dfsrr; @@ -46,7 +46,7 @@ i2c@118100 { #address-cells = <1>; #size-cells = <0>; cell-index = <1>; - compatible = "fsl-i2c"; + compatible = "fsl,mpc8543-i2c", "fsl-i2c"; reg = <0x118100 0x100>; interrupts = <38 2 0 0>; dfsrr; diff --git a/arch/powerpc/boot/dts/fsl/qoriq-i2c-1.dtsi b/arch/powerpc/boot/dts/fsl/qoriq-i2c-1.dtsi index 7989bf5..b697a3b 100644 --- a/arch/powerpc/boot/dts/fsl/qoriq-i2c-1.dtsi +++ b/arch/powerpc/boot/dts/fsl/qoriq-i2c-1.dtsi @@ -36,7 +36,7 @@ i2c@119000 { #address-cells = <1>; #size-cells = <0>; cell-index = <2>; - compatible = "fsl-i2c"; + compatible = "fsl,mpc8543-i2c", "fsl-i2c"; reg = <0x119000 0x100>; interrupts = <39 2 0 0>; dfsrr; @@ -46,7 +46,7 @@ i2c@119100 { #address-cells = <1>; #size-cells = <0>; cell-index = <3>; - compatible = "fsl-i2c"; + compatible = "fsl,mpc8543-i2c", "fsl-i2c"; reg = <0x119100 0x100>; interrupts = <39 2 0 0>; dfsrr;