* [PATCH v2] Set sda-hold-time based on ACPI *CNT value @ 2017-02-14 5:54 chin.yew.tan 2017-02-14 5:54 ` [PATCH] i2c: designware: Get selected speed mode sda-hold-time via ACPI chin.yew.tan 0 siblings, 1 reply; 9+ messages in thread From: chin.yew.tan @ 2017-02-14 5:54 UTC (permalink / raw) To: jarkko.nikula, andriy.shevchenko, mika.westerberg; +Cc: linux-i2c From: Tan Chin Yew <chin.yew.tan@intel.com> For I2c to operate correctly under all speed mode, sda-hold-time need to be perfectly tuned. However, sda-hold-time is precalculated according to circuit parameter which make it platform-specific. In order to get accurate sda-hold-time for all platforms, pretuned sda-hold-time for particular platform is stored in ACPI table and driver to load the sda holding time from ACPI table. This patch read the I2c sda-hold-time from ACPI table and assigned the suitable hold time based on the i2c clock frequency. Tested on Intel Apollo Lake. Changes in V2: - The code is realigned according to suggestion. - "case 400000:" is added on top of "default:" for readability. Tan Chin Yew (1): i2c: designware: Get selected speed mode sda-hold-time via ACPI drivers/i2c/busses/i2c-designware-platdrv.c | 28 +++++++++++++++++++++------- 1 file changed, 21 insertions(+), 7 deletions(-) -- 1.9.1 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH] i2c: designware: Get selected speed mode sda-hold-time via ACPI 2017-02-14 5:54 [PATCH v2] Set sda-hold-time based on ACPI *CNT value chin.yew.tan @ 2017-02-14 5:54 ` chin.yew.tan 2017-02-14 7:00 ` Jarkko Nikula ` (2 more replies) 0 siblings, 3 replies; 9+ messages in thread From: chin.yew.tan @ 2017-02-14 5:54 UTC (permalink / raw) To: jarkko.nikula, andriy.shevchenko, mika.westerberg; +Cc: linux-i2c From: Tan Chin Yew <chin.yew.tan@intel.com> Sda-hold-time is an important parameter for tuning i2c to meet the electrical specification especially for high speed. I2C with incorrect sda-hold-time may cause lost arbitration error. Now, the driver is able to get sda-hold-time for all the speed supported. Signed-off-by: Tan Chin Yew <chin.yew.tan@intel.com> --- drivers/i2c/busses/i2c-designware-platdrv.c | 28 +++++++++++++++++++++------- 1 file changed, 21 insertions(+), 7 deletions(-) diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c index 6ce4313..00c880a 100644 --- a/drivers/i2c/busses/i2c-designware-platdrv.c +++ b/drivers/i2c/busses/i2c-designware-platdrv.c @@ -101,14 +101,28 @@ static int dw_i2c_acpi_configure(struct platform_device *pdev) dev->rx_fifo_depth = 32; /* - * Try to get SDA hold time and *CNT values from an ACPI method if - * it exists for both supported speed modes. + * Try to get SDA hold time and *CNT values from an ACPI method for + * selected speed modes. */ - dw_i2c_acpi_params(pdev, "SSCN", &dev->ss_hcnt, &dev->ss_lcnt, NULL); - dw_i2c_acpi_params(pdev, "FMCN", &dev->fs_hcnt, &dev->fs_lcnt, - &dev->sda_hold_time); - dw_i2c_acpi_params(pdev, "FPCN", &dev->fp_hcnt, &dev->fp_lcnt, NULL); - dw_i2c_acpi_params(pdev, "HSCN", &dev->hs_hcnt, &dev->hs_lcnt, NULL); + switch (dev->clk_freq) { + case 100000: + dw_i2c_acpi_params(pdev, "SSCN", &dev->ss_hcnt, &dev->ss_lcnt, + &dev->sda_hold_time); + break; + case 1000000: + dw_i2c_acpi_params(pdev, "FPCN", &dev->fp_hcnt, &dev->fp_lcnt, + &dev->sda_hold_time); + break; + case 3400000: + dw_i2c_acpi_params(pdev, "HSCN", &dev->hs_hcnt, &dev->hs_lcnt, + &dev->sda_hold_time); + break; + case 400000: + default: + dw_i2c_acpi_params(pdev, "FMCN", &dev->fs_hcnt, &dev->fs_lcnt, + &dev->sda_hold_time); + break; + } id = acpi_match_device(pdev->dev.driver->acpi_match_table, &pdev->dev); if (id && id->driver_data) -- 1.9.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] i2c: designware: Get selected speed mode sda-hold-time via ACPI 2017-02-14 5:54 ` [PATCH] i2c: designware: Get selected speed mode sda-hold-time via ACPI chin.yew.tan @ 2017-02-14 7:00 ` Jarkko Nikula 2017-03-06 12:30 ` Tan, Chin Yew 2017-02-14 10:05 ` Andy Shevchenko 2017-03-23 20:58 ` Wolfram Sang 2 siblings, 1 reply; 9+ messages in thread From: Jarkko Nikula @ 2017-02-14 7:00 UTC (permalink / raw) To: chin.yew.tan, andriy.shevchenko, mika.westerberg; +Cc: linux-i2c On 14.02.2017 07:54, chin.yew.tan@intel.com wrote: > From: Tan Chin Yew <chin.yew.tan@intel.com> > > Sda-hold-time is an important parameter for tuning i2c to meet the > electrical specification especially for high speed. I2C with incorrect > sda-hold-time may cause lost arbitration error. Now, the driver is able to > get sda-hold-time for all the speed supported. > > Signed-off-by: Tan Chin Yew <chin.yew.tan@intel.com> > --- > drivers/i2c/busses/i2c-designware-platdrv.c | 28 +++++++++++++++++++++------- > 1 file changed, 21 insertions(+), 7 deletions(-) > Acked-by: Jarkko Nikula <jarkko.nikula@linux.intel.com> ^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: [PATCH] i2c: designware: Get selected speed mode sda-hold-time via ACPI 2017-02-14 7:00 ` Jarkko Nikula @ 2017-03-06 12:30 ` Tan, Chin Yew 0 siblings, 0 replies; 9+ messages in thread From: Tan, Chin Yew @ 2017-03-06 12:30 UTC (permalink / raw) To: wsa@the-dreams.de Cc: linux-i2c@vger.kernel.org, Jarkko Nikula, andriy.shevchenko@linux.intel.com, mika.westerberg@linux.intel.com Hi Wolfram, Wondering if you manage to review this patch? It is about loading selected speed mode's sda-hold-time via ACPI. I would like to find out if there are any additional comments? Sincerely Tan Chin Yew > -----Original Message----- > From: Jarkko Nikula [mailto:jarkko.nikula@linux.intel.com] > Sent: Tuesday, February 14, 2017 3:01 PM > To: Tan, Chin Yew <chin.yew.tan@intel.com>; > andriy.shevchenko@linux.intel.com; mika.westerberg@linux.intel.com > Cc: linux-i2c@vger.kernel.org > Subject: Re: [PATCH] i2c: designware: Get selected speed mode sda-hold-time > via ACPI > > On 14.02.2017 07:54, chin.yew.tan@intel.com wrote: > > From: Tan Chin Yew <chin.yew.tan@intel.com> > > > > Sda-hold-time is an important parameter for tuning i2c to meet the > > electrical specification especially for high speed. I2C with incorrect > > sda-hold-time may cause lost arbitration error. Now, the driver is > > able to get sda-hold-time for all the speed supported. > > > > Signed-off-by: Tan Chin Yew <chin.yew.tan@intel.com> > > --- > > drivers/i2c/busses/i2c-designware-platdrv.c | 28 > > +++++++++++++++++++++------- > > 1 file changed, 21 insertions(+), 7 deletions(-) > > > > Acked-by: Jarkko Nikula <jarkko.nikula@linux.intel.com> ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] i2c: designware: Get selected speed mode sda-hold-time via ACPI 2017-02-14 5:54 ` [PATCH] i2c: designware: Get selected speed mode sda-hold-time via ACPI chin.yew.tan 2017-02-14 7:00 ` Jarkko Nikula @ 2017-02-14 10:05 ` Andy Shevchenko 2017-03-23 20:58 ` Wolfram Sang 2 siblings, 0 replies; 9+ messages in thread From: Andy Shevchenko @ 2017-02-14 10:05 UTC (permalink / raw) To: chin.yew.tan, jarkko.nikula, mika.westerberg; +Cc: linux-i2c On Tue, 2017-02-14 at 13:54 +0800, chin.yew.tan@intel.com wrote: > From: Tan Chin Yew <chin.yew.tan@intel.com> > > Sda-hold-time is an important parameter for tuning i2c to meet the > electrical specification especially for high speed. I2C with incorrect > sda-hold-time may cause lost arbitration error. Now, the driver is > able to > get sda-hold-time for all the speed supported. > Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > Signed-off-by: Tan Chin Yew <chin.yew.tan@intel.com> > --- > drivers/i2c/busses/i2c-designware-platdrv.c | 28 > +++++++++++++++++++++------- > 1 file changed, 21 insertions(+), 7 deletions(-) > > diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c > b/drivers/i2c/busses/i2c-designware-platdrv.c > index 6ce4313..00c880a 100644 > --- a/drivers/i2c/busses/i2c-designware-platdrv.c > +++ b/drivers/i2c/busses/i2c-designware-platdrv.c > @@ -101,14 +101,28 @@ static int dw_i2c_acpi_configure(struct > platform_device *pdev) > dev->rx_fifo_depth = 32; > > /* > - * Try to get SDA hold time and *CNT values from an ACPI > method if > - * it exists for both supported speed modes. > + * Try to get SDA hold time and *CNT values from an ACPI > method for > + * selected speed modes. > */ > - dw_i2c_acpi_params(pdev, "SSCN", &dev->ss_hcnt, &dev- > >ss_lcnt, NULL); > - dw_i2c_acpi_params(pdev, "FMCN", &dev->fs_hcnt, &dev- > >fs_lcnt, > - &dev->sda_hold_time); > - dw_i2c_acpi_params(pdev, "FPCN", &dev->fp_hcnt, &dev- > >fp_lcnt, NULL); > - dw_i2c_acpi_params(pdev, "HSCN", &dev->hs_hcnt, &dev- > >hs_lcnt, NULL); > + switch (dev->clk_freq) { > + case 100000: > + dw_i2c_acpi_params(pdev, "SSCN", &dev->ss_hcnt, &dev- > >ss_lcnt, > + &dev->sda_hold_time); > + break; > + case 1000000: > + dw_i2c_acpi_params(pdev, "FPCN", &dev->fp_hcnt, &dev- > >fp_lcnt, > + &dev->sda_hold_time); > + break; > + case 3400000: > + dw_i2c_acpi_params(pdev, "HSCN", &dev->hs_hcnt, &dev- > >hs_lcnt, > + &dev->sda_hold_time); > + break; > + case 400000: > + default: > + dw_i2c_acpi_params(pdev, "FMCN", &dev->fs_hcnt, &dev- > >fs_lcnt, > + &dev->sda_hold_time); > + break; > + } > > id = acpi_match_device(pdev->dev.driver->acpi_match_table, > &pdev->dev); > if (id && id->driver_data) -- Andy Shevchenko <andriy.shevchenko@linux.intel.com> Intel Finland Oy ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] i2c: designware: Get selected speed mode sda-hold-time via ACPI 2017-02-14 5:54 ` [PATCH] i2c: designware: Get selected speed mode sda-hold-time via ACPI chin.yew.tan 2017-02-14 7:00 ` Jarkko Nikula 2017-02-14 10:05 ` Andy Shevchenko @ 2017-03-23 20:58 ` Wolfram Sang 2017-03-23 21:00 ` Wolfram Sang 2 siblings, 1 reply; 9+ messages in thread From: Wolfram Sang @ 2017-03-23 20:58 UTC (permalink / raw) To: chin.yew.tan; +Cc: jarkko.nikula, andriy.shevchenko, mika.westerberg, linux-i2c [-- Attachment #1: Type: text/plain, Size: 706 bytes --] On Tue, Feb 14, 2017 at 01:54:01PM +0800, chin.yew.tan@intel.com wrote: > From: Tan Chin Yew <chin.yew.tan@intel.com> > > Sda-hold-time is an important parameter for tuning i2c to meet the > electrical specification especially for high speed. I2C with incorrect > sda-hold-time may cause lost arbitration error. Now, the driver is able to > get sda-hold-time for all the speed supported. This describes why you change NULL to dev->sda_hold_time. But it doesn't say why you introduce the switch-block instead of populating all fields like it was done before. Furthermore, since now there is no NULL case for dw_i2c_acpi_params() anymore, we can remove NULL handling in that function, or? [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] i2c: designware: Get selected speed mode sda-hold-time via ACPI 2017-03-23 20:58 ` Wolfram Sang @ 2017-03-23 21:00 ` Wolfram Sang 2017-03-27 9:43 ` Tan, Chin Yew 0 siblings, 1 reply; 9+ messages in thread From: Wolfram Sang @ 2017-03-23 21:00 UTC (permalink / raw) To: chin.yew.tan; +Cc: jarkko.nikula, andriy.shevchenko, mika.westerberg, linux-i2c [-- Attachment #1: Type: text/plain, Size: 439 bytes --] > This describes why you change NULL to dev->sda_hold_time. But it doesn't > say why you introduce the switch-block instead of populating all fields > like it was done before. > > Furthermore, since now there is no NULL case for dw_i2c_acpi_params() > anymore, we can remove NULL handling in that function, or? And since designware driver had a few changes meanwhile, can you base the next version on top of i2c/for-next? [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: [PATCH] i2c: designware: Get selected speed mode sda-hold-time via ACPI 2017-03-23 21:00 ` Wolfram Sang @ 2017-03-27 9:43 ` Tan, Chin Yew 2017-03-27 10:07 ` Wolfram Sang 0 siblings, 1 reply; 9+ messages in thread From: Tan, Chin Yew @ 2017-03-27 9:43 UTC (permalink / raw) To: Wolfram Sang Cc: jarkko.nikula@linux.intel.com, andriy.shevchenko@linux.intel.com, mika.westerberg@linux.intel.com, linux-i2c@vger.kernel.org Hi > -----Original Message----- > From: Wolfram Sang [mailto:wsa@the-dreams.de] > Sent: Friday, March 24, 2017 5:01 AM > To: Tan, Chin Yew <chin.yew.tan@intel.com> > Cc: jarkko.nikula@linux.intel.com; andriy.shevchenko@linux.intel.com; > mika.westerberg@linux.intel.com; linux-i2c@vger.kernel.org > Subject: Re: [PATCH] i2c: designware: Get selected speed mode sda-hold-time > via ACPI > > > > This describes why you change NULL to dev->sda_hold_time. But it > > doesn't say why you introduce the switch-block instead of populating > > all fields like it was done before. Instead of loading all speed mode settings, switch block is used to load the required settings for selected speed mode. Furthermore, using switch block re-use existing variable, Whereas, populating all fields introduce new variables. > > > > Furthermore, since now there is no NULL case for dw_i2c_acpi_params() > > anymore, we can remove NULL handling in that function, or? Ok, I will remove the null handling. > > And since designware driver had a few changes meanwhile, can you base the > next version on top of i2c/for-next? > Ok, I will base it to i2c/for-next. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] i2c: designware: Get selected speed mode sda-hold-time via ACPI 2017-03-27 9:43 ` Tan, Chin Yew @ 2017-03-27 10:07 ` Wolfram Sang 0 siblings, 0 replies; 9+ messages in thread From: Wolfram Sang @ 2017-03-27 10:07 UTC (permalink / raw) To: Tan, Chin Yew Cc: jarkko.nikula@linux.intel.com, andriy.shevchenko@linux.intel.com, mika.westerberg@linux.intel.com, linux-i2c@vger.kernel.org [-- Attachment #1: Type: text/plain, Size: 382 bytes --] > > > This describes why you change NULL to dev->sda_hold_time. But it > > > doesn't say why you introduce the switch-block instead of populating > > > all fields like it was done before. > Instead of loading all speed mode settings, switch block is used to load the > required settings for selected speed mode. Thanks. Please add this sentence to the commit message. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2017-03-27 10:07 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-02-14 5:54 [PATCH v2] Set sda-hold-time based on ACPI *CNT value chin.yew.tan 2017-02-14 5:54 ` [PATCH] i2c: designware: Get selected speed mode sda-hold-time via ACPI chin.yew.tan 2017-02-14 7:00 ` Jarkko Nikula 2017-03-06 12:30 ` Tan, Chin Yew 2017-02-14 10:05 ` Andy Shevchenko 2017-03-23 20:58 ` Wolfram Sang 2017-03-23 21:00 ` Wolfram Sang 2017-03-27 9:43 ` Tan, Chin Yew 2017-03-27 10:07 ` Wolfram Sang
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).