| Message ID | 1411032367-20274-2-git-send-email-mika.westerberg@linux.intel.com |
|---|---|
| State | Accepted |
| Headers | show |
On Thu, Sep 18, 2014 at 12:26:07PM +0300, Mika Westerberg wrote: > From: Carl Peng <carlpeng008@gmail.com> > > Add support for AMD version of the DW I2C host controller. The device is > enumerated from ACPI namespace with ACPI ID AMD0010. Because the core > driver needs an input source clock, and this is not an Intel LPSS device > where clocks are provided through drivers/acpi/acpi_lpss.c, we register the > clock ourselves if the clock rate is given in ->driver_data. > > Signed-off-by: Carl Peng <carlpeng008@gmail.com> > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com> > --- Applied to for-next, still wondering... > drivers/i2c/busses/Kconfig | 1 + > drivers/i2c/busses/i2c-designware-platdrv.c | 27 +++++++++++++++++++++++++++ > 2 files changed, 28 insertions(+) > > diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig > index 2ac87fa3058d..9384498ef3c1 100644 > --- a/drivers/i2c/busses/Kconfig > +++ b/drivers/i2c/busses/Kconfig > @@ -422,6 +422,7 @@ config I2C_DESIGNWARE_CORE > > config I2C_DESIGNWARE_PLATFORM > tristate "Synopsys DesignWare Platform" > + depends on COMMON_CLK ... do all previous users have COMMON_CLK?
On Sat, Sep 20, 2014 at 11:36:34AM +0200, Wolfram Sang wrote: > On Thu, Sep 18, 2014 at 12:26:07PM +0300, Mika Westerberg wrote: > > From: Carl Peng <carlpeng008@gmail.com> > > > > Add support for AMD version of the DW I2C host controller. The device is > > enumerated from ACPI namespace with ACPI ID AMD0010. Because the core > > driver needs an input source clock, and this is not an Intel LPSS device > > where clocks are provided through drivers/acpi/acpi_lpss.c, we register the > > clock ourselves if the clock rate is given in ->driver_data. > > > > Signed-off-by: Carl Peng <carlpeng008@gmail.com> > > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com> > > --- > > Applied to for-next, still wondering... Thanks! > > > drivers/i2c/busses/Kconfig | 1 + > > drivers/i2c/busses/i2c-designware-platdrv.c | 27 +++++++++++++++++++++++++++ > > 2 files changed, 28 insertions(+) > > > > diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig > > index 2ac87fa3058d..9384498ef3c1 100644 > > --- a/drivers/i2c/busses/Kconfig > > +++ b/drivers/i2c/busses/Kconfig > > @@ -422,6 +422,7 @@ config I2C_DESIGNWARE_CORE > > > > config I2C_DESIGNWARE_PLATFORM > > tristate "Synopsys DesignWare Platform" > > + depends on COMMON_CLK > > ... do all previous users have COMMON_CLK? The driver is being used on x86, ARM and ARC it seems. For x86 and ARM we pretty much have COMMON_CLK nowadays but I'm not sure about ARC. That's why I have copied Christian Ruppert. Christian, Do you see problems on your side if we depend on COMMON_CLK? -- 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
Dear Mika, On Mon, Sep 22, 2014 at 12:12:07PM +0300, Mika Westerberg wrote: > On Sat, Sep 20, 2014 at 11:36:34AM +0200, Wolfram Sang wrote: > > On Thu, Sep 18, 2014 at 12:26:07PM +0300, Mika Westerberg wrote: > > > From: Carl Peng <carlpeng008@gmail.com> > > > > > > Add support for AMD version of the DW I2C host controller. The device is > > > enumerated from ACPI namespace with ACPI ID AMD0010. Because the core > > > driver needs an input source clock, and this is not an Intel LPSS device > > > where clocks are provided through drivers/acpi/acpi_lpss.c, we register the > > > clock ourselves if the clock rate is given in ->driver_data. > > > > > > Signed-off-by: Carl Peng <carlpeng008@gmail.com> > > > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com> > > > --- > > > > Applied to for-next, still wondering... > > Thanks! > > > > > > drivers/i2c/busses/Kconfig | 1 + > > > drivers/i2c/busses/i2c-designware-platdrv.c | 27 +++++++++++++++++++++++++++ > > > 2 files changed, 28 insertions(+) > > > > > > diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig > > > index 2ac87fa3058d..9384498ef3c1 100644 > > > --- a/drivers/i2c/busses/Kconfig > > > +++ b/drivers/i2c/busses/Kconfig > > > @@ -422,6 +422,7 @@ config I2C_DESIGNWARE_CORE > > > > > > config I2C_DESIGNWARE_PLATFORM > > > tristate "Synopsys DesignWare Platform" > > > + depends on COMMON_CLK > > > > ... do all previous users have COMMON_CLK? > > The driver is being used on x86, ARM and ARC it seems. For x86 and ARM > we pretty much have COMMON_CLK nowadays but I'm not sure about ARC. > That's why I have copied Christian Ruppert. > > Christian, > > Do you see problems on your side if we depend on COMMON_CLK? COMMON_CLK is not selected by the ARC architecture in general. However, we do select COMMON_CLK in the TB10x platform which uses the designware I2C driver so this new dependency is no problem for us. Vineet, Do you see any issues with this on other existing ARC platforms, e.g. arcfpga? Greetings, Christian -- 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 Monday 22 September 2014 05:59 PM, Christian Ruppert wrote: > Dear Mika, > > On Mon, Sep 22, 2014 at 12:12:07PM +0300, Mika Westerberg wrote: >> On Sat, Sep 20, 2014 at 11:36:34AM +0200, Wolfram Sang wrote: >>> On Thu, Sep 18, 2014 at 12:26:07PM +0300, Mika Westerberg wrote: >>>> From: Carl Peng <carlpeng008@gmail.com> >>>> >>>> Add support for AMD version of the DW I2C host controller. The device is >>>> enumerated from ACPI namespace with ACPI ID AMD0010. Because the core >>>> driver needs an input source clock, and this is not an Intel LPSS device >>>> where clocks are provided through drivers/acpi/acpi_lpss.c, we register the >>>> clock ourselves if the clock rate is given in ->driver_data. >>>> >>>> Signed-off-by: Carl Peng <carlpeng008@gmail.com> >>>> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com> >>>> --- >>> Applied to for-next, still wondering... >> Thanks! >> >>>> drivers/i2c/busses/Kconfig | 1 + >>>> drivers/i2c/busses/i2c-designware-platdrv.c | 27 +++++++++++++++++++++++++++ >>>> 2 files changed, 28 insertions(+) >>>> >>>> diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig >>>> index 2ac87fa3058d..9384498ef3c1 100644 >>>> --- a/drivers/i2c/busses/Kconfig >>>> +++ b/drivers/i2c/busses/Kconfig >>>> @@ -422,6 +422,7 @@ config I2C_DESIGNWARE_CORE >>>> >>>> config I2C_DESIGNWARE_PLATFORM >>>> tristate "Synopsys DesignWare Platform" >>>> + depends on COMMON_CLK >>> ... do all previous users have COMMON_CLK? >> The driver is being used on x86, ARM and ARC it seems. For x86 and ARM >> we pretty much have COMMON_CLK nowadays but I'm not sure about ARC. >> That's why I have copied Christian Ruppert. >> >> Christian, >> >> Do you see problems on your side if we depend on COMMON_CLK? > COMMON_CLK is not selected by the ARC architecture in general. However, > we do select COMMON_CLK in the TB10x platform which uses the designware > I2C driver so this new dependency is no problem for us. > > Vineet, > > Do you see any issues with this on other existing ARC platforms, e.g. > arcfpga? So what needs to be done, COMMON_CLK needs to be defined in arch/arc/Kconfig ? And if so why ? OTOH, if we do have to, I don't see the issue with it - we just didn't need it for the legacy platform. -Vineet -- 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, Sep 22, 2014 at 01:48:57PM +0000, Vineet Gupta wrote: > On Monday 22 September 2014 05:59 PM, Christian Ruppert wrote: > > Dear Mika, > > > > On Mon, Sep 22, 2014 at 12:12:07PM +0300, Mika Westerberg wrote: > >> On Sat, Sep 20, 2014 at 11:36:34AM +0200, Wolfram Sang wrote: > >>> On Thu, Sep 18, 2014 at 12:26:07PM +0300, Mika Westerberg wrote: > >>>> From: Carl Peng <carlpeng008@gmail.com> > >>>> > >>>> Add support for AMD version of the DW I2C host controller. The device is > >>>> enumerated from ACPI namespace with ACPI ID AMD0010. Because the core > >>>> driver needs an input source clock, and this is not an Intel LPSS device > >>>> where clocks are provided through drivers/acpi/acpi_lpss.c, we register the > >>>> clock ourselves if the clock rate is given in ->driver_data. > >>>> > >>>> Signed-off-by: Carl Peng <carlpeng008@gmail.com> > >>>> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com> > >>>> --- > >>> Applied to for-next, still wondering... > >> Thanks! > >> > >>>> drivers/i2c/busses/Kconfig | 1 + > >>>> drivers/i2c/busses/i2c-designware-platdrv.c | 27 +++++++++++++++++++++++++++ > >>>> 2 files changed, 28 insertions(+) > >>>> > >>>> diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig > >>>> index 2ac87fa3058d..9384498ef3c1 100644 > >>>> --- a/drivers/i2c/busses/Kconfig > >>>> +++ b/drivers/i2c/busses/Kconfig > >>>> @@ -422,6 +422,7 @@ config I2C_DESIGNWARE_CORE > >>>> > >>>> config I2C_DESIGNWARE_PLATFORM > >>>> tristate "Synopsys DesignWare Platform" > >>>> + depends on COMMON_CLK > >>> ... do all previous users have COMMON_CLK? > >> The driver is being used on x86, ARM and ARC it seems. For x86 and ARM > >> we pretty much have COMMON_CLK nowadays but I'm not sure about ARC. > >> That's why I have copied Christian Ruppert. > >> > >> Christian, > >> > >> Do you see problems on your side if we depend on COMMON_CLK? > > COMMON_CLK is not selected by the ARC architecture in general. However, > > we do select COMMON_CLK in the TB10x platform which uses the designware > > I2C driver so this new dependency is no problem for us. > > > > Vineet, > > > > Do you see any issues with this on other existing ARC platforms, e.g. > > arcfpga? > > So what needs to be done, COMMON_CLK needs to be defined in arch/arc/Kconfig ? And > if so why ? Without COMMON_CLK, you are not able to select I2C_DESIGNWARE_PLATFORM anymore. So if something on ARC depends on this driver then we either need the COMMON_CLK there or figure out alternative way to fix Carl's problem. > OTOH, if we do have to, I don't see the issue with it - we just didn't need it > for the legacy platform. Is the legacy platform using this driver? Grepping "i2c_designware" returns zero board files so at least that should be covered. -- 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 Monday 22 September 2014 07:30 PM, Mika Westerberg wrote: >>> COMMON_CLK is not selected by the ARC architecture in general. However, >>> > > we do select COMMON_CLK in the TB10x platform which uses the designware >>> > > I2C driver so this new dependency is no problem for us. >>> > > >>> > > Vineet, >>> > > >>> > > Do you see any issues with this on other existing ARC platforms, e.g. >>> > > arcfpga? >> > >> > So what needs to be done, COMMON_CLK needs to be defined in arch/arc/Kconfig ? And >> > if so why ? > Without COMMON_CLK, you are not able to select I2C_DESIGNWARE_PLATFORM > anymore. So if something on ARC depends on this driver then we either > need the COMMON_CLK there or figure out alternative way to fix Carl's > problem. I have not seen the orig patch, but it seems COMMON_CLK is already being selected by TB10x, do we still need it in arch/arcKconfig, for all ARC platforms ? >> > OTOH, if we do have to, I don't see the issue with it - we just didn't need it >> > for the legacy platform. > Is the legacy platform using this driver? Grepping "i2c_designware" > returns zero board files so at least that should be covered. > -- 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, Sep 22, 2014 at 02:16:25PM +0000, Vineet Gupta wrote: > On Monday 22 September 2014 07:30 PM, Mika Westerberg wrote: > >>> COMMON_CLK is not selected by the ARC architecture in general. However, > >>> > > we do select COMMON_CLK in the TB10x platform which uses the designware > >>> > > I2C driver so this new dependency is no problem for us. > >>> > > > >>> > > Vineet, > >>> > > > >>> > > Do you see any issues with this on other existing ARC platforms, e.g. > >>> > > arcfpga? > >> > > >> > So what needs to be done, COMMON_CLK needs to be defined in arch/arc/Kconfig ? And > >> > if so why ? > > Without COMMON_CLK, you are not able to select I2C_DESIGNWARE_PLATFORM > > anymore. So if something on ARC depends on this driver then we either > > need the COMMON_CLK there or figure out alternative way to fix Carl's > > problem. > > I have not seen the orig patch, but it seems COMMON_CLK is already being selected > by TB10x, do we still need it in arch/arcKconfig, for all ARC platforms ? Only if the driver is being used by something else than TB10x on ARC. So that we don't cause regressions to existing users. -- 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, Sep 22, 2014 at 02:16:25PM +0000, Vineet Gupta wrote: > On Monday 22 September 2014 07:30 PM, Mika Westerberg wrote: > >>> COMMON_CLK is not selected by the ARC architecture in general. However, > >>> > > we do select COMMON_CLK in the TB10x platform which uses the designware > >>> > > I2C driver so this new dependency is no problem for us. > >>> > > > >>> > > Vineet, > >>> > > > >>> > > Do you see any issues with this on other existing ARC platforms, e.g. > >>> > > arcfpga? > >> > > >> > So what needs to be done, COMMON_CLK needs to be defined in arch/arc/Kconfig ? And > >> > if so why ? > > Without COMMON_CLK, you are not able to select I2C_DESIGNWARE_PLATFORM > > anymore. So if something on ARC depends on this driver then we either > > need the COMMON_CLK there or figure out alternative way to fix Carl's > > problem. > > I have not seen the orig patch, but it seems COMMON_CLK is already being selected > by TB10x, do we still need it in arch/arcKconfig, for all ARC platforms ? Sorry for the confusion, I should have given you some context. Mika has checked that designware i2c is used by some ARC platforms but he didn't say which ones. The orig patch makes COMMON_CLK a requirement for designware i2c. I checked that we're fine for TB10x (we need COMMON_CLK anyway) but since you weren't in copy I just wanted to make sure that none of the other ARC platforms (which don't seem to select COMMON_CLK) use designware i2c and thus run into trouble. If there is no designware i2c in any of your platforms, simply ignore my message. From my point of view there is no need to move "select COMMON_CLK" up to the ARC architecture level. Greetings, Christian
On Mon, Sep 22, 2014 at 07:22:52PM +0200, Christian Ruppert wrote: > On Mon, Sep 22, 2014 at 02:16:25PM +0000, Vineet Gupta wrote: > > On Monday 22 September 2014 07:30 PM, Mika Westerberg wrote: > > >>> COMMON_CLK is not selected by the ARC architecture in general. However, > > >>> > > we do select COMMON_CLK in the TB10x platform which uses the designware > > >>> > > I2C driver so this new dependency is no problem for us. > > >>> > > > > >>> > > Vineet, > > >>> > > > > >>> > > Do you see any issues with this on other existing ARC platforms, e.g. > > >>> > > arcfpga? > > >> > > > >> > So what needs to be done, COMMON_CLK needs to be defined in arch/arc/Kconfig ? And > > >> > if so why ? > > > Without COMMON_CLK, you are not able to select I2C_DESIGNWARE_PLATFORM > > > anymore. So if something on ARC depends on this driver then we either > > > need the COMMON_CLK there or figure out alternative way to fix Carl's > > > problem. > > > > I have not seen the orig patch, but it seems COMMON_CLK is already being selected > > by TB10x, do we still need it in arch/arcKconfig, for all ARC platforms ? > > Sorry for the confusion, I should have given you some context. Mika > has checked that designware i2c is used by some ARC platforms but he > didn't say which ones. The orig patch makes COMMON_CLK a requirement for > designware i2c. Yeah, sorry about not mentioning the affected platforms. I've included them now in this email. > I checked that we're fine for TB10x (we need COMMON_CLK anyway) but > since you weren't in copy I just wanted to make sure that none of the > other ARC platforms (which don't seem to select COMMON_CLK) use > designware i2c and thus run into trouble. > > If there is no designware i2c in any of your platforms, simply ignore > my message. From my point of view there is no need to move "select > COMMON_CLK" up to the ARC architecture level. I grepped for snps,i2c-designware from the kernel source tree and this is what I found: Only ARC that is using the driver: arch/arc/boot/dts/abilis_tb10x.dtsi -> OK Two ARMs that pass clock to the driver: arch/arm/boot/dts/berlin2q.dtsi -> OK arch/arm/boot/dts/socfpga.dts -> OK Following files have the designware driver listed but it seems to be disabled and does not get any clocks: arch/arm/boot/dts/nspire-cx.dts arch/arm/boot/dts/spear1310.dtsi arch/arm/boot/dts/spear320.dtsi arch/arm/boot/dts/spear3xx.dtsi arch/arm/boot/dts/spear600.dtsi I think we are good to go with the first three but not sure about the last five. Adding Viresh Kumar and Daniel Tang to the thread. Viresh, Daniel, The thread is here http://patchwork.ozlabs.org/patch/390695/. In summary, we are adding COMMON_CLK dependency to the i2c-designware-platdrv.c in order to get clocks to one AMD DW based host controller and wanted to check that we don't break the existing users (nspire and SPEAR). -- 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
Hi Chrisitian, On Monday 22 September 2014 10:53 PM, Christian Ruppert wrote: > If there is no designware i2c in any of your platforms, simply ignore > my message. From my point of view there is no need to move "select > COMMON_CLK" up to the ARC architecture level. I still ended up moving COMMON_CLK out of tb10x to arch/arc - due to consolidation of .init_machine() callbacks - from platform to arc core. Give linux-next a spin (perhaps later today as I just pushed a build error in that area). You'll like the almost empty platform file :-) -Vineet -- 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, Sep 23, 2014 at 3:05 AM, Mika Westerberg <mika.westerberg@linux.intel.com> wrote: > arch/arm/boot/dts/spear1310.dtsi > arch/arm/boot/dts/spear320.dtsi > arch/arm/boot/dts/spear3xx.dtsi > arch/arm/boot/dts/spear600.dtsi > > I think we are good to go with the first three but not sure about the > last five. Adding Viresh Kumar and Daniel Tang to the thread. > > Viresh, Daniel, > > The thread is here http://patchwork.ozlabs.org/patch/390695/. > > In summary, we are adding COMMON_CLK dependency to the > i2c-designware-platdrv.c in order to get clocks to one AMD DW based host > controller and wanted to check that we don't break the existing users > (nspire and SPEAR). A dependency on COMMON_CLK is fine for SPEAr . -- 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, Sep 22, 2014 at 12:12:07PM +0300, Mika Westerberg wrote: > On Sat, Sep 20, 2014 at 11:36:34AM +0200, Wolfram Sang wrote: > > On Thu, Sep 18, 2014 at 12:26:07PM +0300, Mika Westerberg wrote: > > > From: Carl Peng <carlpeng008@gmail.com> > > > > > > Add support for AMD version of the DW I2C host controller. The device is > > > enumerated from ACPI namespace with ACPI ID AMD0010. Because the core > > > driver needs an input source clock, and this is not an Intel LPSS device > > > where clocks are provided through drivers/acpi/acpi_lpss.c, we register the > > > clock ourselves if the clock rate is given in ->driver_data. > > > > > > Signed-off-by: Carl Peng <carlpeng008@gmail.com> > > > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com> > > > --- > > > > Applied to for-next, still wondering... > > Thanks! I reconsidered and these two patches are not in i2c/for-next because of two reasons: 1) They do not apply cleanly on top of other i2c-designware changes I applied (check i2c/for-next I just pushed out). The conflicts might not be hard, but they were not trivial enough for me to do them inbetween other things. I'd be very happy about a repost on top of i2c/for-next. > > > > > > drivers/i2c/busses/Kconfig | 1 + > > > drivers/i2c/busses/i2c-designware-platdrv.c | 27 +++++++++++++++++++++++++++ > > > 2 files changed, 28 insertions(+) > > > > > > diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig > > > index 2ac87fa3058d..9384498ef3c1 100644 > > > --- a/drivers/i2c/busses/Kconfig > > > +++ b/drivers/i2c/busses/Kconfig > > > @@ -422,6 +422,7 @@ config I2C_DESIGNWARE_CORE > > > > > > config I2C_DESIGNWARE_PLATFORM > > > tristate "Synopsys DesignWare Platform" > > > + depends on COMMON_CLK > > > > ... do all previous users have COMMON_CLK? > > The driver is being used on x86, ARM and ARC it seems. For x86 and ARM > we pretty much have COMMON_CLK nowadays but I'm not sure about ARC. 2) "pretty much have" is not so convincing to me. That is such a generic core, it probably has enough out-of-tree users as well. And with all these ACPI regressions from this cycle, I am very cautious right now. Brainstorming: What about "depends on (ACPI && COMMON_CLK) || !ACPI"?
On Mon, Sep 29, 2014 at 11:24:50PM +0200, Wolfram Sang wrote: > On Mon, Sep 22, 2014 at 12:12:07PM +0300, Mika Westerberg wrote: > > On Sat, Sep 20, 2014 at 11:36:34AM +0200, Wolfram Sang wrote: > > > On Thu, Sep 18, 2014 at 12:26:07PM +0300, Mika Westerberg wrote: > > > > From: Carl Peng <carlpeng008@gmail.com> > > > > > > > > Add support for AMD version of the DW I2C host controller. The device is > > > > enumerated from ACPI namespace with ACPI ID AMD0010. Because the core > > > > driver needs an input source clock, and this is not an Intel LPSS device > > > > where clocks are provided through drivers/acpi/acpi_lpss.c, we register the > > > > clock ourselves if the clock rate is given in ->driver_data. > > > > > > > > Signed-off-by: Carl Peng <carlpeng008@gmail.com> > > > > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com> > > > > --- > > > > > > Applied to for-next, still wondering... > > > > Thanks! > > I reconsidered and these two patches are not in i2c/for-next because of > two reasons: > > 1) They do not apply cleanly on top of other i2c-designware changes I > applied (check i2c/for-next I just pushed out). The conflicts might not > be hard, but they were not trivial enough for me to do them inbetween > other things. I'd be very happy about a repost on top of i2c/for-next. OK, I'll do that. > > > > > > > drivers/i2c/busses/Kconfig | 1 + > > > > drivers/i2c/busses/i2c-designware-platdrv.c | 27 +++++++++++++++++++++++++++ > > > > 2 files changed, 28 insertions(+) > > > > > > > > diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig > > > > index 2ac87fa3058d..9384498ef3c1 100644 > > > > --- a/drivers/i2c/busses/Kconfig > > > > +++ b/drivers/i2c/busses/Kconfig > > > > @@ -422,6 +422,7 @@ config I2C_DESIGNWARE_CORE > > > > > > > > config I2C_DESIGNWARE_PLATFORM > > > > tristate "Synopsys DesignWare Platform" > > > > + depends on COMMON_CLK > > > > > > ... do all previous users have COMMON_CLK? > > > > The driver is being used on x86, ARM and ARC it seems. For x86 and ARM > > we pretty much have COMMON_CLK nowadays but I'm not sure about ARC. > > 2) "pretty much have" is not so convincing to me. That is such a generic > core, it probably has enough out-of-tree users as well. I wasn't aware that we care about out-of-tree users. > And with all these ACPI regressions from this cycle, I am very > cautious right now. Brainstorming: What about "depends on (ACPI && > COMMON_CLK) || !ACPI"? Understood. I'll try the above. Thanks. -- 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
> > 1) They do not apply cleanly on top of other i2c-designware changes I > > applied (check i2c/for-next I just pushed out). The conflicts might not > > be hard, but they were not trivial enough for me to do them inbetween > > other things. I'd be very happy about a repost on top of i2c/for-next. > > OK, I'll do that. Great, much appreciated! > > 2) "pretty much have" is not so convincing to me. That is such a generic > > core, it probably has enough out-of-tree users as well. > > I wasn't aware that we care about out-of-tree users. This is one example coming from a feeling that we should at least try to make this change having less impact. > > And with all these ACPI regressions from this cycle, I am very > > cautious right now. Brainstorming: What about "depends on (ACPI && > > COMMON_CLK) || !ACPI"? > > Understood. I'll try the above. Cool, thanks!
Hi Vineet, On Fri, Sep 26, 2014 at 09:20:20AM +0530, Vineet Gupta wrote: > Hi Chrisitian, > > On Monday 22 September 2014 10:53 PM, Christian Ruppert wrote: > > If there is no designware i2c in any of your platforms, simply ignore > > my message. From my point of view there is no need to move "select > > COMMON_CLK" up to the ARC architecture level. > > I still ended up moving COMMON_CLK out of tb10x to arch/arc - due to consolidation > of .init_machine() callbacks - from platform to arc core. > > Give linux-next a spin (perhaps later today as I just pushed a build error in that > area). > > You'll like the almost empty platform file :-) This looks very nice indeed. Support for "standard" platforms is simple and elegant. The fact that most platforms won't need any specific support code at all (apart from drivers) goes well with the idea of device tree. However, the kernel-doc comment for init_machine in mach_desc.h is now slightly confusing (still mentioning device tree). With this patch there remains only a single detail we need to manage through platform-specific code: the reset handler. Today we still provide a patch for the machine_restart function in reset.c to our customers so that rebooting from the command line works. Do you have any plans/ideas to fix this one as well? Greetings, Christian -- 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
+CC Guenter Roeck On Tuesday 07 October 2014 03:00 PM, Christian Ruppert wrote: > Hi Vineet, > > On Fri, Sep 26, 2014 at 09:20:20AM +0530, Vineet Gupta wrote: >> Hi Chrisitian, >> >> On Monday 22 September 2014 10:53 PM, Christian Ruppert wrote: >>> If there is no designware i2c in any of your platforms, simply ignore >>> my message. From my point of view there is no need to move "select >>> COMMON_CLK" up to the ARC architecture level. >> I still ended up moving COMMON_CLK out of tb10x to arch/arc - due to consolidation >> of .init_machine() callbacks - from platform to arc core. >> >> Give linux-next a spin (perhaps later today as I just pushed a build error in that >> area). >> >> You'll like the almost empty platform file :-) > This looks very nice indeed. Support for "standard" platforms is simple > and elegant. The fact that most platforms won't need any specific > support code at all (apart from drivers) goes well with the idea of > device tree. However, the kernel-doc comment for init_machine in > mach_desc.h is now slightly confusing (still mentioning device tree). A platform of future can still call of_platform_populate() etc to reparse the stuff for say it's platform devices ! So I would think it is still relevant ! > With this patch there remains only a single detail we need to manage > through platform-specific code: the reset handler. Today we still > provide a patch for the machine_restart function in reset.c to our > customers so that rebooting from the command line works. Do you have any > plans/ideas to fix this one as well? Patches are welcome ;-) ATM, I dont have a specific use-case for my current platforms, so can't write the code - you can propose a patch and then we can work out what's best in general for all ARC platforms. BTW there's a series in flight on related topic from Guenter so please take a look at that too for big picture ! http://www.spinics.net/linux/lists/kernel/msg1840650.html Cheers, -Vineet > > Greetings, > Christian > -- 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, Oct 07, 2014 at 12:35:29PM +0000, Vineet Gupta wrote: > +CC Guenter Roeck > [...] > > However, the kernel-doc comment for init_machine in > > mach_desc.h is now slightly confusing (still mentioning device tree). > > A platform of future can still call of_platform_populate() etc to reparse the > stuff for say it's platform devices ! > So I would think it is still relevant ! OK. > > With this patch there remains only a single detail we need to manage > > through platform-specific code: the reset handler. Today we still > > provide a patch for the machine_restart function in reset.c to our > > customers so that rebooting from the command line works. Do you have any > > plans/ideas to fix this one as well? > > Patches are welcome ;-) > > ATM, I dont have a specific use-case for my current platforms, so can't write the > code - you can propose a patch and then we can work out what's best in general for > all ARC platforms. BTW there's a series in flight on related topic from Guenter so > please take a look at that too for big picture ! > > http://www.spinics.net/linux/lists/kernel/msg1840650.html Actually, before sending my previous mail I looked at the current implementation of the halt hook and didn't like it (otherwise I would have proposed something in the lines). So this one is definitely a step forward! I'm wondering about two things concerning reset, however: 1. Is the PM module the right place for a reset handler? On the one hand reset is functionally very similar to power off but on the other hand reset is technically not a power management functionality. If the PM module is not the right place, which would be the right place instead? 2. What would be the desired behaviour/semantics for a reset handler chain equivalent to the power off handler chain. I see two possibilities here: a) Implementation exactly like power off. Every handler is expected to reset the entire system and never returns. b) A more "modular" implementation where different subsystems are being reset sequentially (e.g. first reset peripherals through GPIO in "high priority" handlers and finally reset the core in the terminal "low priority" handler). Greetings, Christian -- 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/10/2014 02:13 AM, Christian Ruppert wrote: > On Tue, Oct 07, 2014 at 12:35:29PM +0000, Vineet Gupta wrote: >> +CC Guenter Roeck >> [...] >>> However, the kernel-doc comment for init_machine in >>> mach_desc.h is now slightly confusing (still mentioning device tree). >> >> A platform of future can still call of_platform_populate() etc to reparse the >> stuff for say it's platform devices ! >> So I would think it is still relevant ! > > OK. > >>> With this patch there remains only a single detail we need to manage >>> through platform-specific code: the reset handler. Today we still >>> provide a patch for the machine_restart function in reset.c to our >>> customers so that rebooting from the command line works. Do you have any >>> plans/ideas to fix this one as well? >> >> Patches are welcome ;-) >> >> ATM, I dont have a specific use-case for my current platforms, so can't write the >> code - you can propose a patch and then we can work out what's best in general for >> all ARC platforms. BTW there's a series in flight on related topic from Guenter so >> please take a look at that too for big picture ! >> >> http://www.spinics.net/linux/lists/kernel/msg1840650.html > > Actually, before sending my previous mail I looked at the current > implementation of the halt hook and didn't like it (otherwise I would > have proposed something in the lines). So this one is definitely a step > forward! I'm wondering about two things concerning reset, however: > > 1. Is the PM module the right place for a reset handler? On the one hand > reset is functionally very similar to power off but on the other hand > reset is technically not a power management functionality. If the PM > module is not the right place, which would be the right place > instead? > > 2. What would be the desired behaviour/semantics for a reset handler > chain equivalent to the power off handler chain. I see two > possibilities here: > a) Implementation exactly like power off. Every handler is expected > to reset the entire system and never returns. > b) A more "modular" implementation where different subsystems are > being reset sequentially (e.g. first reset peripherals through > GPIO in "high priority" handlers and finally reset the core in the > terminal "low priority" handler). > Hi Christian, The restart handler patch series will hopefully make it into 3.18. It will not be in kernel/power/ but in kernel/reboot.c which seemed to be more appropriate. The semantics is exactly the same as for the poweroff handler. Actually, the poweroff handler code is derived from the restart handler code. Implementation is like 2a). The restart handler code is available here: https://git.kernel.org/cgit/linux/kernel/git/groeck/linux-staging.git/log/?h=restart-handler There is also a tag, restart-handler-for-v3.18, which you can use to merge the series into your tree if needed. Hope this helps, Guenter -- 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/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig index 2ac87fa3058d..9384498ef3c1 100644 --- a/drivers/i2c/busses/Kconfig +++ b/drivers/i2c/busses/Kconfig @@ -422,6 +422,7 @@ config I2C_DESIGNWARE_CORE config I2C_DESIGNWARE_PLATFORM tristate "Synopsys DesignWare Platform" + depends on COMMON_CLK select I2C_DESIGNWARE_CORE help If you say yes to this option, support will be included for the diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c index 7087b6ee97e2..ecd0215d93c2 100644 --- a/drivers/i2c/busses/i2c-designware-platdrv.c +++ b/drivers/i2c/busses/i2c-designware-platdrv.c @@ -30,6 +30,7 @@ #include <linux/delay.h> #include <linux/i2c.h> #include <linux/clk.h> +#include <linux/clk-provider.h> #include <linux/errno.h> #include <linux/sched.h> #include <linux/err.h> @@ -80,6 +81,7 @@ static int dw_i2c_acpi_configure(struct platform_device *pdev) { struct dw_i2c_dev *dev = platform_get_drvdata(pdev); bool fs_mode = dev->master_cfg & DW_IC_CON_SPEED_FAST; + const struct acpi_device_id *id; dev->adapter.nr = -1; dev->tx_fifo_depth = 32; @@ -94,9 +96,29 @@ static int dw_i2c_acpi_configure(struct platform_device *pdev) dw_i2c_acpi_params(pdev, "FMCN", &dev->fs_hcnt, &dev->fs_lcnt, fs_mode ? &dev->sda_hold_time : NULL); + /* + * Provide a way for Designware I2C host controllers that are not + * based on Intel LPSS to specify their input clock frequency via + * ->driver_data. + */ + id = acpi_match_device(pdev->dev.driver->acpi_match_table, &pdev->dev); + if (id && id->driver_data) + clk_register_fixed_rate(&pdev->dev, dev_name(&pdev->dev), NULL, + CLK_IS_ROOT, id->driver_data); + return 0; } +static void dw_i2c_acpi_unconfigure(struct platform_device *pdev) +{ + struct dw_i2c_dev *dev = platform_get_drvdata(pdev); + const struct acpi_device_id *id; + + id = acpi_match_device(pdev->dev.driver->acpi_match_table, &pdev->dev); + if (id && id->driver_data) + clk_unregister(dev->clk); +} + static const struct acpi_device_id dw_i2c_acpi_match[] = { { "INT33C2", 0 }, { "INT33C3", 0 }, @@ -104,6 +126,7 @@ static const struct acpi_device_id dw_i2c_acpi_match[] = { { "INT3433", 0 }, { "80860F41", 0 }, { "808622C1", 0 }, + { "AMD0010", 133 * 1000 * 1000 }, { } }; MODULE_DEVICE_TABLE(acpi, dw_i2c_acpi_match); @@ -112,6 +135,7 @@ static inline int dw_i2c_acpi_configure(struct platform_device *pdev) { return -ENODEV; } +static inline void dw_i2c_acpi_unconfigure(struct platform_device *pdev) { } #endif static int dw_i2c_probe(struct platform_device *pdev) @@ -235,6 +259,9 @@ static int dw_i2c_remove(struct platform_device *pdev) pm_runtime_put(&pdev->dev); pm_runtime_disable(&pdev->dev); + if (ACPI_COMPANION(&pdev->dev)) + dw_i2c_acpi_unconfigure(pdev); + return 0; }