Re: [RESEND PATCH 0/9] eeprom: at24: at24cs series serial number read

[Bartosz Golaszewski]

2015-10-21 16:23 GMT+02:00 Peter Korsgaard :
>> "Bartosz" == Bartosz Golaszewski  writes:
>
>  >> As the serial number is available on a separate i2c address, wouldn't
>  >> it be simpler to handle these as special (read only) device variants and
>  >> instantiate E.G. a 24c64 (for the normal data) and a 24cs64 (for the
>  >> serial)?
>  >>
>
>  > Hi Peter,
>
>  > I wanted to respond that this way we would not be protected from
>  > concurrent accesses, but then I saw I didn't actually include any
>  > locks in the serial read function - my bad. It needs to be fixed as
>  > both memory blocks share the same address pointer.
>
>  > I'll resend the series.
>
> But we're protected by the i2c bus lock, right? You do a single
> i2c_transfer to read the serial number.

Why the at24->lock then?

Best regards,
Bartosz Golaszewski
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 2/2] i2c: at91: manage unexpected RXRDY flag when starting a transfer

[Ludovic Desroches]

In some cases, we could start a new i2c transfer with the RXRDY flag
set. It is not a clean state and it leads to print annoying error
messages even if there no real issue. The cause is only having garbage
data in the Receive Holding Register because of a weird behavior of the
RXRDY flag.

Signed-off-by: Ludovic Desroches 
Fixes: 93563a6a71bb ("i2c: at91: fix a race condition when using the DMA
controller")
Reported-by: Peter Rosin 
Tested-by: Peter Rosin 
Cc: sta...@vger.kernel.org #4.1
---
 drivers/i2c/busses/i2c-at91.c | 33 +
 1 file changed, 25 insertions(+), 8 deletions(-)

diff --git a/drivers/i2c/busses/i2c-at91.c b/drivers/i2c/busses/i2c-at91.c
index 94c087b..bac0016 100644
--- a/drivers/i2c/busses/i2c-at91.c
+++ b/drivers/i2c/busses/i2c-at91.c
@@ -347,8 +347,14 @@ error:
 
 static void at91_twi_read_next_byte(struct at91_twi_dev *dev)
 {
-   if (!dev->buf_len)
+   /*
+* If we are in this case, it means there is garbage data in RHR, so
+* delete them.
+*/
+   if (!dev->buf_len) {
+   at91_twi_read(dev, AT91_TWI_RHR);
return;
+   }
 
/* 8bit read works with and without FIFO */
*dev->buf = readb_relaxed(dev->base + AT91_TWI_RHR);
@@ -465,6 +471,24 @@ static irqreturn_t atmel_twi_interrupt(int irq, void 
*dev_id)
 
if (!irqstatus)
return IRQ_NONE;
+   /*
+* In reception, the behavior of the twi device (before sama5d2) is
+* weird. There is some magic about RXRDY flag! When a data has been
+* almost received, the reception of a new one is anticipated if there
+* is no stop command to send. That is the reason why ask for sending
+* the stop command not on the last data but on the second last one.
+*
+* Unfortunately, we could still have the RXRDY flag set even if the
+* transfer is done and we have read the last data. It might happen
+* when the i2c slave device sends too quickly data after receiving the
+* ack from the master. The data has been almost received before having
+* the order to send stop. In this case, sending the stop command could
+* cause a RXRDY interrupt with a TXCOMP one. It is better to manage
+* the RXRDY interrupt first in order to not keep garbage data in the
+* Receive Holding Register for the next transfer.
+*/
+   if (irqstatus & AT91_TWI_RXRDY)
+   at91_twi_read_next_byte(dev);
 
/*
 * When a NACK condition is detected, the I2C controller sets the NACK,
@@ -507,8 +531,6 @@ static irqreturn_t atmel_twi_interrupt(int irq, void 
*dev_id)
if (irqstatus & (AT91_TWI_TXCOMP | AT91_TWI_NACK)) {
at91_disable_twi_interrupts(dev);
complete(>cmd_complete);
-   } else if (irqstatus & AT91_TWI_RXRDY) {
-   at91_twi_read_next_byte(dev);
} else if (irqstatus & AT91_TWI_TXRDY) {
at91_twi_write_next_byte(dev);
}
@@ -600,11 +622,6 @@ static int at91_do_twi_transfer(struct at91_twi_dev *dev)
} else if (dev->msg->flags & I2C_M_RD) {
unsigned start_flags = AT91_TWI_START;
 
-   if (sr & AT91_TWI_RXRDY) {
-   dev_err(dev->dev, "RXRDY still set!");
-   at91_twi_read(dev, AT91_TWI_RHR);
-   }
-
/* if only one byte is to be read, immediately stop transfer */
if (!has_alt_cmd && dev->buf_len <= 1 &&
!(dev->msg->flags & I2C_M_RECV_LEN))
-- 
2.5.0

--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 1/2] i2c: at91: fix write transfers by clearing pending interrupt first

[Ludovic Desroches]

From: Cyrille Pitchen 

In some cases a NACK interrupt may be pending in the Status Register (SR)
as a result of a previous transfer. However at91_do_twi_transfer() did not
read the SR to clear pending interruptions before starting a new transfer.
Hence a NACK interrupt rose as soon as it was enabled again at the I2C
controller level, resulting in a wrong sequence of operations and strange
patterns of behaviour on the I2C bus, such as a clock stretch followed by
a restart of the transfer.

This first issue occurred with both DMA and PIO write transfers.

Also when a NACK error was detected during a PIO write transfer, the
interrupt handler used to wrongly start a new transfer by writing into the
Transmit Holding Register (THR). Then the I2C slave was likely to reply
with a second NACK.

This second issue is fixed in atmel_twi_interrupt() by handling the TXRDY
status bit only if both the TXCOMP and NACK status bits are cleared.

Tested with a at24 eeprom on sama5d36ek board running a linux-4.1-at91
kernel image. Adapted to linux-next.

Signed-off-by: Cyrille Pitchen 
Fixes: 93563a6a71bb ("i2c: at91: fix a race condition when using the DMA 
controller")
Reported-by: Peter Rosin 
Signed-off-by: Ludovic Desroches 
Tested-by: Peter Rosin 
Cc: sta...@vger.kernel.org #4.1
---
 drivers/i2c/busses/i2c-at91.c | 58 +--
 1 file changed, 50 insertions(+), 8 deletions(-)

diff --git a/drivers/i2c/busses/i2c-at91.c b/drivers/i2c/busses/i2c-at91.c
index 1c758cd..94c087b 100644
--- a/drivers/i2c/busses/i2c-at91.c
+++ b/drivers/i2c/busses/i2c-at91.c
@@ -465,19 +465,57 @@ static irqreturn_t atmel_twi_interrupt(int irq, void 
*dev_id)
 
if (!irqstatus)
return IRQ_NONE;
-   else if (irqstatus & AT91_TWI_RXRDY)
-   at91_twi_read_next_byte(dev);
-   else if (irqstatus & AT91_TWI_TXRDY)
-   at91_twi_write_next_byte(dev);
-
-   /* catch error flags */
-   dev->transfer_status |= status;
 
+   /*
+* When a NACK condition is detected, the I2C controller sets the NACK,
+* TXCOMP and TXRDY bits all together in the Status Register (SR).
+*
+* 1 - Handling NACK errors with CPU write transfer.
+*
+* In such case, we should not write the next byte into the Transmit
+* Holding Register (THR) otherwise the I2C controller would start a new
+* transfer and the I2C slave is likely to reply by another NACK.
+*
+* 2 - Handling NACK errors with DMA write transfer.
+*
+* By setting the TXRDY bit in the SR, the I2C controller also triggers
+* the DMA controller to write the next data into the THR. Then the
+* result depends on the hardware version of the I2C controller.
+*
+* 2a - Without support of the Alternative Command mode.
+*
+* This is the worst case: the DMA controller is triggered to write the
+* next data into the THR, hence starting a new transfer: the I2C slave
+* is likely to reply by another NACK.
+* Concurrently, this interrupt handler is likely to be called to manage
+* the first NACK before the I2C controller detects the second NACK and
+* sets once again the NACK bit into the SR.
+* When handling the first NACK, this interrupt handler disables the I2C
+* controller interruptions, especially the NACK interrupt.
+* Hence, the NACK bit is pending into the SR. This is why we should
+* read the SR to clear all pending interrupts at the beginning of
+* at91_do_twi_transfer() before actually starting a new transfer.
+*
+* 2b - With support of the Alternative Command mode.
+*
+* When a NACK condition is detected, the I2C controller also locks the
+* THR (and sets the LOCK bit in the SR): even though the DMA controller
+* is triggered by the TXRDY bit to write the next data into the THR,
+* this data actually won't go on the I2C bus hence a second NACK is not
+* generated.
+*/
if (irqstatus & (AT91_TWI_TXCOMP | AT91_TWI_NACK)) {
at91_disable_twi_interrupts(dev);
complete(>cmd_complete);
+   } else if (irqstatus & AT91_TWI_RXRDY) {
+   at91_twi_read_next_byte(dev);
+   } else if (irqstatus & AT91_TWI_TXRDY) {
+   at91_twi_write_next_byte(dev);
}
 
+   /* catch error flags */
+   dev->transfer_status |= status;
+
return IRQ_HANDLED;
 }
 
@@ -487,6 +525,7 @@ static int at91_do_twi_transfer(struct at91_twi_dev *dev)
unsigned long time_left;
bool has_unre_flag = dev->pdata->has_unre_flag;
bool has_alt_cmd = dev->pdata->has_alt_cmd;
+   unsigned sr;
 
/*
 * WARNING: the TXCOMP bit in the 

Re: [RESEND PATCH 0/9] eeprom: at24: at24cs series serial number read

[Peter Korsgaard]

> "Bartosz" == Bartosz Golaszewski  writes:

Hi,

 >> > I wanted to respond that this way we would not be protected from
 >> > concurrent accesses, but then I saw I didn't actually include any
 >> > locks in the serial read function - my bad. It needs to be fixed as
 >> > both memory blocks share the same address pointer.
 >> 
 >> > I'll resend the series.
 >> 
 >> But we're protected by the i2c bus lock, right? You do a single
 >> i2c_transfer to read the serial number.

 > Why the at24->lock then?

I'm not sure. Wolfram?

-- 
Bye, Peter Korsgaard 
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 6/6] i2c: designware: Move common probe code into i2c_dw_probe()

[Jarkko Nikula]

Hi

On 10/20/2015 07:32 PM, Wolfram Sang wrote:

There was a merge conflict with a bugfix from i2c/for-current. I think
it is okay, but you may want to double check my i2c/for-next.

Looks like pm_runtime_disable() got dropped from your 36d48fb5766a 
("i2c: designware-platdrv: enable RuntimePM before registering to the 
core") while handling the merge conflict. I'll send a fix.



What about this irq-clearing-in-probe thingie on top of this series? :)

I'll have a look at it. What's not entirely clear to me would it be 
no-op or not. HW is actually disabled after i2c_dw_init() which is 
called before requesting the interrupt but is not clear to me from the 
spec does HW clear interrupts while it goes idle.


So as a result I'd expect either a explicit interrupt clearing patch (to 
be more robust against potential unmasking changes) or a comment in 
__i2c_dw_enable() :-)


--
Jarkko
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RESEND PATCH 0/9] eeprom: at24: at24cs series serial number read

[Peter Korsgaard]

> "Bartosz" == Bartosz Golaszewski  writes:

 >> As the serial number is available on a separate i2c address, wouldn't
 >> it be simpler to handle these as special (read only) device variants and
 >> instantiate E.G. a 24c64 (for the normal data) and a 24cs64 (for the
 >> serial)?
 >> 

 > Hi Peter,

 > I wanted to respond that this way we would not be protected from
 > concurrent accesses, but then I saw I didn't actually include any
 > locks in the serial read function - my bad. It needs to be fixed as
 > both memory blocks share the same address pointer.

 > I'll resend the series.

But we're protected by the i2c bus lock, right? You do a single
i2c_transfer to read the serial number.

-- 
Venlig hilsen,
Peter Korsgaard 
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] i2c: designware: Disable runtime PM in case i2c_dw_probe() fails

[Jarkko Nikula]

Call to pm_runtime_disable() got dropped while handling the merge conflict
between commit 36d48fb5766a ("i2c: designware-platdrv: enable RuntimePM
before registering to the core") and commit d80d134182ba ("i2c: designware:
Move common probe code into i2c_dw_probe()").

Signed-off-by: Jarkko Nikula 
---
 drivers/i2c/busses/i2c-designware-platdrv.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c 
b/drivers/i2c/busses/i2c-designware-platdrv.c
index eb55760ccd9f..8fd9d4a18bd5 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -261,8 +261,10 @@ static int dw_i2c_plat_probe(struct platform_device *pdev)
}
 
r = i2c_dw_probe(dev);
-   if (r)
+   if (r) {
+   pm_runtime_disable(>dev);
return r;
+   }
 
return 0;
 }
-- 
2.6.1

--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 0/6] enable I2C devices behind I2C bus on Gen2

[Andy Shevchenko]

On Wed, 2015-10-07 at 13:18 +0300, Andy Shevchenko wrote:
> There is a board in the wild, i.e. Intel Galileo Gen2, that has ACPI
> enumerated
> devices behind I2C bus.

Lee, since Wolfram is going to apply patches 1 and 5, how could we
proceed with the rest? Patches are indeed build independent, though
they are unified by enabling logically piece-by-piece. It would be
great if you think you may apply them to your v4.4 queue.


> This patch series dedicated to enable those devices. Meanwhile it
> also changes
> I2C core to cope with ACPI 6.0 specification (patch 1).
> 
> The MFD framework is also updated to cope with interesting
> implementation of
> the cell descriptions under ACPI MFD (patch 2).
> 
> The patches 5 and 6 are pretty independent and could be applied
> ahead, though
> they don't make much sense without previous ones.
> 
> Srinivas, it would be nice to see your tag (ideally Tested-by) to be
> sure we
> don't break ISH stuff.
> 
> Since it touches multiple subsystems someone needs to create an
> immutable
> branch. I don't actually know whose subsystem better here. Wolfram?
> 
> Tested on the actual Intel Galileo Gen2 by Ismo (gpio expanders) and
> me (at24).
> 
> Changelog v3:
> - append ACKs from Rafael (from ACPI angle)
> - drop upstreamed patches (GPIO pca953x)
> 
> Changelog v2:
> - append tags
> - re-make patch 3 (suggested by Lee)
> - improve patch 8 (suggested by Thierry)
> 
> Andy Shevchenko (5):
>   mfd: core: redo ACPI matching of the children devices
>   mfd: intel_quark_i2c_gpio: load gpio driver first
>   mfd: intel_quark_i2c_gpio: support devices behind i2c bus
>   at24: enable ACPI device found on Galileo Gen2
>   pwm-pca9685: enable ACPI device found on Galileo Gen2
> 
> Mika Westerberg (1):
>   i2c / ACPI: Rework I2C device scanning
> 
>  Documentation/acpi/enumeration.txt | 11 +++--
>  drivers/i2c/i2c-core.c | 82 +++-
> --
>  drivers/mfd/intel_quark_i2c_gpio.c | 33 ++-
>  drivers/mfd/mfd-core.c | 52 
>  drivers/misc/eeprom/at24.c | 22 --
>  drivers/pwm/Kconfig|  2 +-
>  drivers/pwm/pwm-pca9685.c  | 20 --
>  include/linux/mfd/core.h   | 10 -
>  8 files changed, 170 insertions(+), 62 deletions(-)
> 

-- 
Andy Shevchenko 
Intel Finland Oy

--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: linux-next: Tree for Oct 21 (drivers/i2c/busses/i2c-designware-platdrv.c)

[Jarkko Nikula]

Hi

On 10/21/2015 11:28 PM, Randy Dunlap wrote:

On 10/20/15 23:16, Stephen Rothwell wrote:

Hi all,

There will be no linux-next releases after tomorrow until Nov 2 (kernel
summit).

Changes since 20151020:



on i386 or x86_64:

when CONFIG_PM_SLEEP is not enabled:

../drivers/i2c/busses/i2c-designware-platdrv.c:340:13: error: 
'dw_i2c_plat_prepare' undeclared here (not in a function)
   .prepare = dw_i2c_plat_prepare,
  ^
../drivers/i2c/busses/i2c-designware-platdrv.c:341:14: error: 
'dw_i2c_plat_complete' undeclared here (not in a function)
   .complete = dw_i2c_plat_complete,
   ^

Here's the fix for this:

http://marc.info/?l=linux-i2c=144541136917692=2

--
Jarkko
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [GIT PULL] On-demand device probing

[Frank Rowand]

On 10/19/2015 5:34 AM, Tomeu Vizoso wrote:
> On 18 October 2015 at 21:53, Mark Brown  wrote:
>> On Sun, Oct 18, 2015 at 12:37:57PM -0700, Greg Kroah-Hartman wrote:
>>> On Sun, Oct 18, 2015 at 08:29:31PM +0100, Mark Brown wrote:
 On Fri, Oct 16, 2015 at 11:57:50PM -0700, Greg Kroah-Hartman wrote:

< snip >

> hope you don't mind I summarize the points taken instead of replying
> to the individual emails. I tried to address all the concerns that
> have been raised again in the cover letter, but I guess I did a bad
> job at explaining myself, so here's another (more in-depth) go at it.

< snip >

> 3) Regarding total boot time, I don't expect this series to make much
> of a difference because though we would save a lot of matching and
> querying for resources, that's little time compared with how long we
> wait for hardware to react during probing. Async probing is more
> likely to help with drivers that take a long time to probe.

And then in your reply to Russell's reply to your email you say:

> To be clear, I was saying that this series should NOT affect total
> boot times much.

I'm confused.  If I understood correctly, improving boot time was
the key justification for accepting this patch set.  For example,
from "[PATCH v7 0/20] On-demand device probing":

   I have a problem with the panel on my Tegra Chromebook taking longer
   than expected to be ready during boot (Stéphane Marchesin reported what
   is basically the same issue in [0]), and have looked into ordered
   probing as a better way of solving this than moving nodes around in the
   DT or playing with initcall levels and linking order.

   ...

   With this series I get the kernel to output to the panel in 0.5s,
   instead of 2.8s.

Alexander Holler reported improved boot times for his patch set
in August, which is another approach to ordering probes
(http://article.gmane.org/gmane.linux.drivers.devicetree/133010).
His results for 5 boards was four booted faster, one slightly
slower:

   Some numbers (5 boots on each board, without and with ordering drivers),
   all times are seconds.

   Kirkwood (dockstar, armv5):

   Boot to "Freeing unused kernel memory" (includes mounting the rootfs),
   unordered:
   4.456016 3.937801 4.114788 4.114526 3.949480 (average 4.1145222)
   ordered:
   3.173054 3.164045 3.141418 3.480679 3.459298 (3.2836988)
   Time needed to sort (of_init_build_order()):
   0.003024
   Time needed to match drivers to the order (without calling them):
   0.002884

   Beagleboard (rev C4, armv7):

   unordered:
   6.706024 6.821746 6.696014 6.673675 6.769866 (6.733465)
   ordered:
   5.544860 5.514160 5.505859 5.527374 5.496795 (5.5178096)
   sorting: 0.021209
   matching: 0.006165

   Beaglebone Black (rev A5, armv7):

   unordered:
   3.826531 3.825662 3.826648 3.825434 3.825263 (3.8259076)
   ordered:
   2.838554 2.838322 2.839459 2.838467 2.838421 (2.8386446)
   sorting: 0.004769
   matching: 0.004860

   imx6q (armv7):

   unordered:
   3.451998 3.418864 3.446952 3.429974 3.440996 (3.4377568)
   ordered:
   3.538312 3.549019 3.538105 3.515916 3.555715 (3.5394134)
   sorting: 0.004622
   matching: 0.003868

While not as dramatic as your results, they are somewhat supportive.
What has changed your assessment that the on-demand device probing
patches will give a big boot performance increase?  Do you have
new data or analysis?

-Frank
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [GIT PULL] On-demand device probing

[Mark Brown]

On Wed, Oct 21, 2015 at 08:59:51AM -0700, Frank Rowand wrote:
> On 10/19/2015 5:34 AM, Tomeu Vizoso wrote:

> > To be clear, I was saying that this series should NOT affect total
> > boot times much.

> I'm confused.  If I understood correctly, improving boot time was
> the key justification for accepting this patch set.  For example,
> from "[PATCH v7 0/20] On-demand device probing":
> 
>I have a problem with the panel on my Tegra Chromebook taking longer
>than expected to be ready during boot (Stéphane Marchesin reported what
>is basically the same issue in [0]), and have looked into ordered
>probing as a better way of solving this than moving nodes around in the
>DT or playing with initcall levels and linking order.
> 
>...
> 
>With this series I get the kernel to output to the panel in 0.5s,
>instead of 2.8s.

Overall boot time and time to get some individual built in component up
and running aren't the same thing - what this'll do is get things up
more in the link order of the leaf consumers rather than deferring those
leaf consumers when their dependencies aren't ready yet.

> While not as dramatic as your results, they are somewhat supportive.
> What has changed your assessment that the on-demand device probing
> patches will give a big boot performance increase?  Do you have
> new data or analysis?

See above, my understanding was that the performance improvements were
more around improved control/predictability/handwave of the boot
ordering rather than total time.


signature.asc
Description: PGP signature

Re: [PATCH v2] dts: keystone: Use new "ti,keystone-i2c" compatible

[santosh.shilim...@oracle.com]

On 10/21/15 3:38 AM, Grygorii Strashko wrote:

On 09/14/2015 12:07 PM, Alexander Sverdlin wrote:

Now as "i2c-davinci" driver has special handling for Keystone it's
time to switch
the device tree to use new "compatible" property. Old one is left for
backwards-
compatibility.

Signed-off-by: Alexander Sverdlin 
---



Will pick it up. Thanks !!

Regards,
Santosh

--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Alternative approach to solve the deferred probe

[Frank Rowand]

On 10/21/2015 1:18 AM, Russell King - ARM Linux wrote:
> On Tue, Oct 20, 2015 at 08:58:19PM -0700, Frank Rowand wrote:
>> On 10/20/2015 8:46 AM, Russell King - ARM Linux wrote:

< snip >

>>> +
>>>  static bool driver_deferred_probe_enable = false;
>>> +
>>>  /**
>>>   * driver_deferred_probe_trigger() - Kick off re-probing deferred devices
>>>   *
>>> @@ -188,6 +210,13 @@ static int deferred_probe_initcall(void)
>>> driver_deferred_probe_trigger();
>>
>> Couldn't you put the "driver_deferred_probe_report = true" here?  And then
>> not add another round of probes.
> 
> The idea is not to report anything for drivers that were deferred
> during the normal bootup.  The above is part of the normal bootup,
> and the deferred activity should not be warned about.

The above is currently the last point for probe to succeed or defer
(until possibly, as you mentioned, module loading resolves the defer).
If a probe defers above, it will defer again below.  The set of defers
should be exactly the same above and below.

> 
> If we have any devices still deferring after _this_ round, that must
> indicate that some resource they want is not available, and that
> should be warned about.
> 
> Of course, modules can defer too - and I made some suggestions in my
> waffle above the patch about that.
> 

< adding back trimmed, for fuller context >

>>> /* Sort as many dependencies as possible before exiting initcalls */
>>> flush_workqueue(deferred_wq);
>>> +
>>> +   /* Now one final round, reporting any devices that remain deferred */
>>> +   driver_deferred_probe_report = true;
>>> +   driver_deferred_probe_trigger();
>>> +   /* Sort as many dependencies as possible before exiting initcalls */
>>> +   flush_workqueue(deferred_wq);
>>> +
>>> return 0;
>>>  }
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Alternative approach to solve the deferred probe

[Grygorii Strashko]

On 10/21/2015 06:36 PM, Frank Rowand wrote:
> On 10/21/2015 1:18 AM, Russell King - ARM Linux wrote:
>> On Tue, Oct 20, 2015 at 08:58:19PM -0700, Frank Rowand wrote:
>>> On 10/20/2015 8:46 AM, Russell King - ARM Linux wrote:
> 
> < snip >
> 
 +
   static bool driver_deferred_probe_enable = false;
 +
   /**
* driver_deferred_probe_trigger() - Kick off re-probing deferred devices
*
 @@ -188,6 +210,13 @@ static int deferred_probe_initcall(void)
driver_deferred_probe_trigger();
>>>
>>> Couldn't you put the "driver_deferred_probe_report = true" here?  And then
>>> not add another round of probes.
>>
>> The idea is not to report anything for drivers that were deferred
>> during the normal bootup.  The above is part of the normal bootup,
>> and the deferred activity should not be warned about.
> 
> The above is currently the last point for probe to succeed or defer
> (until possibly, as you mentioned, module loading resolves the defer).
> If a probe defers above, it will defer again below.  The set of defers
> should be exactly the same above and below.
> 

Unfortunately this is not "the last point for probe to succeed or defer".
There are still a bunch of drivers in Kernel which will be probed at 
late_initcall() level.
(like ./drivers/net/ethernet/ti/cpsw.c => late_initcall(cpsw_init);
Yes - they probably need to be updated to use module_init(), but that's what
we have now). Those drivers will re-trigger deferred device probing if their
probe succeeded.

As result, it is impossible to say when will it happen the 
"final round of deferred device probing" :( and final list of drivers which
was "deferred forever" will be know only when kernel exits to User space 
("deferred forever" - before loading modules).

May be, we also can consider adding debug_fs entry which can be used to display
actual state of deferred_probe_pending_list? 

>>
>> If we have any devices still deferring after _this_ round, that must
>> indicate that some resource they want is not available, and that
>> should be warned about.
>>
>> Of course, modules can defer too - and I made some suggestions in my
>> waffle above the patch about that.
>>
> 
> < adding back trimmed, for fuller context >
> 
/* Sort as many dependencies as possible before exiting initcalls */
flush_workqueue(deferred_wq);
 +
 +  /* Now one final round, reporting any devices that remain deferred */
 +  driver_deferred_probe_report = true;
 +  driver_deferred_probe_trigger();
 +  /* Sort as many dependencies as possible before exiting initcalls */
 +  flush_workqueue(deferred_wq);
 +
return 0;
   }


-- 
regards,
-grygorii
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RESEND PATCH 0/9] eeprom: at24: at24cs series serial number read

[Bartosz Golaszewski]

2015-10-21 13:03 GMT+02:00 Peter Korsgaard :
>> "Bartosz" == Bartosz Golaszewski  writes:
>
>  > Chips from the at24cs EEPROM series have an additional read-only memory 
> area
>  > containing a factory pre-programmed serial number. In order to access it, a
>  > dummy write must be executed before reading the serial number bytes.
>
>  > This series adds support for reading the serial number through a sysfs
>  > attribute.
>
>  > While we're at it: some of the patches contain readability tweaks and code
>  > organization fixes.
>
>  > Tested with at24cs64 and at24cs02 chips (for both 16 and 8 bit address
>  > pointers).
>
> As the serial number is available on a separate i2c address, wouldn't
> it be simpler to handle these as special (read only) device variants and
> instantiate E.G. a 24c64 (for the normal data) and a 24cs64 (for the
> serial)?
>

Hi Peter,

I wanted to respond that this way we would not be protected from
concurrent accesses, but then I saw I didn't actually include any
locks in the serial read function - my bad. It needs to be fixed as
both memory blocks share the same address pointer.

I'll resend the series.

Best regards,
Bartosz Golaszewski
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Alternative approach to solve the deferred probe

[Russell King - ARM Linux]

On Wed, Oct 21, 2015 at 07:55:29PM +0300, Grygorii Strashko wrote:
> On 10/21/2015 06:36 PM, Frank Rowand wrote:
> > The above is currently the last point for probe to succeed or defer
> > (until possibly, as you mentioned, module loading resolves the defer).
> > If a probe defers above, it will defer again below.  The set of defers
> > should be exactly the same above and below.
> > 
> 
> Unfortunately this is not "the last point for probe to succeed or defer".

Of course it isn't.  Being pedantic, there's actually no such thing,
because the point that the kernel as finished booting can never actually
be determined with things like modules being present.  That's something
I've acknowledged from the start of this.

> There are still a bunch of drivers in Kernel which will be probed at 
> late_initcall() level.
> (like ./drivers/net/ethernet/ti/cpsw.c => late_initcall(cpsw_init);
> Yes - they probably need to be updated to use module_init(), but that's what
> we have now). Those drivers will re-trigger deferred device probing if their
> probe succeeded.

Maybe this particular late_initcall() which triggers off the deferred
probing should be moved to its own really_late_initcall() which happens
as the very last thing - I think this is intended to run after everything
else has had a chance to probe once.

> As result, it is impossible to say when will it happen the 
> "final round of deferred device probing" :( and final list of drivers
> which was "deferred forever" will be know only when kernel exits to
> User space  ("deferred forever" - before loading modules).
> 
> May be, we also can consider adding debug_fs entry which can be used to
> display actual state of deferred_probe_pending_list? 

There are complaints in this thread about the existing deferred probing
implementation being hard to debug - where it's known that a device
has deferred, but it's not known why that happened.

That would be solved by my proposal, as this final round of probing
before entering userspace after _all_ normal device probes have been
attempted once and then we've tried to satisfy the deferred probe
(okay, that's what it's _supposed_ to be - and as it takes three lines
to write it, you'll excuse me if I just use the abbreviated "final
round of deferred probe" which is much shorter - but remember that
the long version is what I actually mean) would produce a list of
not only the devices that failed to probe, but also the cause of the
deferred probes.

My proposal would ensure that subsystems are happier to add these
prints, because in the normal scenario where we have deferred probing,
we're not littering the console log with lots of useless failure
messages which make people stop and think "now did device X probe?"
It also means scripts in our boot farms can more effectively analyse
the log and determine whether the boot was actually successful and
contained no errors.

Merely printing the list of devices which have been deferred is next
to useless.  The next question will always be "why did device X defer?"
and if that can't be answered, it means people having to spend a long
time adding lots of printks to the kernel at lots of -EPROBE_DEFER
returning sites or in the relevant drivers, tracing through the code
back towards the -EPROBE_DEFER sites to try and track it down.

-- 
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Alternative approach to solve the deferred probe

[Frank Rowand]

On 10/21/2015 9:55 AM, Grygorii Strashko wrote:
> On 10/21/2015 06:36 PM, Frank Rowand wrote:
>> On 10/21/2015 1:18 AM, Russell King - ARM Linux wrote:
>>> On Tue, Oct 20, 2015 at 08:58:19PM -0700, Frank Rowand wrote:
 On 10/20/2015 8:46 AM, Russell King - ARM Linux wrote:
>>
>> < snip >
>>
> +
>   static bool driver_deferred_probe_enable = false;
> +
>   /**
>* driver_deferred_probe_trigger() - Kick off re-probing deferred 
> devices
>*
> @@ -188,6 +210,13 @@ static int deferred_probe_initcall(void)
>   driver_deferred_probe_trigger();

 Couldn't you put the "driver_deferred_probe_report = true" here?  And then
 not add another round of probes.
>>>
>>> The idea is not to report anything for drivers that were deferred
>>> during the normal bootup.  The above is part of the normal bootup,
>>> and the deferred activity should not be warned about.
>>
>> The above is currently the last point for probe to succeed or defer
>> (until possibly, as you mentioned, module loading resolves the defer).
>> If a probe defers above, it will defer again below.  The set of defers
>> should be exactly the same above and below.
>>
> 
> Unfortunately this is not "the last point for probe to succeed or defer".
> There are still a bunch of drivers in Kernel which will be probed at 
> late_initcall() level.
> (like ./drivers/net/ethernet/ti/cpsw.c => late_initcall(cpsw_init);

Yes, cpsw_init() should _not_ be a late_initcall.  This is yet another
example of playing games with ordering probes that we have been trying
to eliminate.

Thanks for pointing out one of the resulting problems this causes for the
deferred probe mechanism.

> Yes - they probably need to be updated to use module_init(), but that's what
> we have now). Those drivers will re-trigger deferred device probing if their
> probe succeeded.

Yes, if cpsw_init() leads to a successful probe, then deferred device probing
will be re-triggered.  I do not know if cpsw_init() will be called before or
after deferred_probe_initcall().  The general initcall mechanism does not
provide any ordering guarantees between the two functions because they are
at the same initcall level.

> 
> As result, it is impossible to say when will it happen the 
> "final round of deferred device probing" :( and final list of drivers which
> was "deferred forever" will be know only when kernel exits to User space 
> ("deferred forever" - before loading modules).
> 
> May be, we also can consider adding debug_fs entry which can be used to 
> display
> actual state of deferred_probe_pending_list? 
> 
>>>
>>> If we have any devices still deferring after _this_ round, that must
>>> indicate that some resource they want is not available, and that
>>> should be warned about.
>>>
>>> Of course, modules can defer too - and I made some suggestions in my
>>> waffle above the patch about that.
>>>
>>
>> < adding back trimmed, for fuller context >
>>
>   /* Sort as many dependencies as possible before exiting 
> initcalls */
>   flush_workqueue(deferred_wq);
> +
> + /* Now one final round, reporting any devices that remain deferred */
> + driver_deferred_probe_report = true;
> + driver_deferred_probe_trigger();
> + /* Sort as many dependencies as possible before exiting initcalls */
> + flush_workqueue(deferred_wq);
> +
>   return 0;
>   }
> 
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [GIT PULL] On-demand device probing

[Frank Rowand]

On 10/21/2015 9:27 AM, Mark Brown wrote:
> On Wed, Oct 21, 2015 at 08:59:51AM -0700, Frank Rowand wrote:
>> On 10/19/2015 5:34 AM, Tomeu Vizoso wrote:
> 
>>> To be clear, I was saying that this series should NOT affect total
>>> boot times much.
> 
>> I'm confused.  If I understood correctly, improving boot time was
>> the key justification for accepting this patch set.  For example,
>> from "[PATCH v7 0/20] On-demand device probing":
>>
>>I have a problem with the panel on my Tegra Chromebook taking longer
>>than expected to be ready during boot (Stéphane Marchesin reported what
>>is basically the same issue in [0]), and have looked into ordered
>>probing as a better way of solving this than moving nodes around in the
>>DT or playing with initcall levels and linking order.
>>
>>...
>>
>>With this series I get the kernel to output to the panel in 0.5s,
>>instead of 2.8s.
> 
> Overall boot time and time to get some individual built in component up
> and running aren't the same thing - what this'll do is get things up
> more in the link order of the leaf consumers rather than deferring those
> leaf consumers when their dependencies aren't ready yet.

Thanks!  I read too much into what was being improved.

So this patch series, which on other merits may be a good idea, is as
a by product solving a specific ordering issue, moving successful panel
initialization to an earlier point in the boot sequence, if I now
understand more correctly.

In that context, this seems like yet another ad hoc way of causing the
probe order to change in a way to solves one specific issue?  Could
it just as likely move the boot order of some other driver on some
other board later, to the detriment of somebody else?


> 
>> While not as dramatic as your results, they are somewhat supportive.
>> What has changed your assessment that the on-demand device probing
>> patches will give a big boot performance increase?  Do you have
>> new data or analysis?
> 
> See above, my understanding was that the performance improvements were
> more around improved control/predictability/handwave of the boot
> ordering rather than total time.
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Alternative approach to solve the deferred probe

[Russell King - ARM Linux]

On Wed, Oct 21, 2015 at 09:13:48PM +0300, Grygorii Strashko wrote:
> But I worry a bit (and that my main point) about these few additional
> rounds of deferred device probing which I have right now and which allows
> some of drivers to finish, finally, their probes successfully.
> With proposed change I'll get more messages in boot log, but some of
> them will belong to drivers which have been probed successfully and so,
> they will be not really useful.

Then you haven't properly understood my proposal.

I want to get rid of all the "X deferred its probing" messages up until
the point that we set the "please report deferred probes" flag.

That _should_ mean that all the deferred probing that goes on becomes
_totally_ silent and becomes hidden (unless you really want to see it,
in which case we can make a debug option which turns it on) up until
we're at the point where we want to enter userspace.

At that point, we then report into the kernel log which devices are
still deferring and, via appropriately placed dev_warn_deferred(),
the reasons why the devices are being deferred.

So, gone will be all the messages earlier in the log about device X
not having a GPIO/clock/whatever because the device providing the
GPIO/clock/whatever hasn't been probed.

If everything is satisfied by the time we run this last round (again,
I'm not using a three line sentence to describe exactly what I mean,
I'm sure you know by now... oops, I just did) then the kernel will
report nothing about any deferrals.  That's _got_ to be an improvement.

> 
> As result, I think, the most important thing is to identify (or create)
> some point during kernel boot when it will be possible to say that all
> built-in drivers (at least) finish their probes 100% (done or defer).
> 
> Might be do_initcalls() can be updated (smth like this):
> static void __init do_initcalls(void)
> {
>   int level;
> 
>   for (level = 0; level < ARRAY_SIZE(initcall_levels) - 1; level++)
>   do_initcall_level(level);
> 
> + wait_for_device_probe();
> + /* Now one final round, reporting any devices that remain deferred */
> + driver_deferred_probe_report = true;
> + driver_deferred_probe_trigger();
> + wait_for_device_probe();
> }
> 
> Also, in my opinion, it will be useful if this debugging feature will be
> optional.

I wonder why you want it optional... so I'm going to guess and cover
both cases I can think of below to head off another round of reply on
this point (sorry if this sucks eggs.)

I don't see it as being optional, because it's going to be cheap to run
in the case of a system which has very few or no errors - which is what
you should have for production systems, right?

Remember, only devices and drivers that are present and have been
probed once get added to the deferred probe list, not devices for
which their drivers are modules.

-- 
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Alternative approach to solve the deferred probe

[Grygorii Strashko]

On 10/21/2015 09:02 PM, Frank Rowand wrote:
> On 10/21/2015 9:55 AM, Grygorii Strashko wrote:
>> On 10/21/2015 06:36 PM, Frank Rowand wrote:
>>> On 10/21/2015 1:18 AM, Russell King - ARM Linux wrote:
 On Tue, Oct 20, 2015 at 08:58:19PM -0700, Frank Rowand wrote:
> On 10/20/2015 8:46 AM, Russell King - ARM Linux wrote:
>>>
>>> < snip >
>>>
>> +
>>static bool driver_deferred_probe_enable = false;
>> +
>>/**
>> * driver_deferred_probe_trigger() - Kick off re-probing deferred 
>> devices
>> *
>> @@ -188,6 +210,13 @@ static int deferred_probe_initcall(void)
>>  driver_deferred_probe_trigger();
>
> Couldn't you put the "driver_deferred_probe_report = true" here?  And then
> not add another round of probes.

 The idea is not to report anything for drivers that were deferred
 during the normal bootup.  The above is part of the normal bootup,
 and the deferred activity should not be warned about.
>>>
>>> The above is currently the last point for probe to succeed or defer
>>> (until possibly, as you mentioned, module loading resolves the defer).
>>> If a probe defers above, it will defer again below.  The set of defers
>>> should be exactly the same above and below.
>>>
>>
>> Unfortunately this is not "the last point for probe to succeed or defer".
>> There are still a bunch of drivers in Kernel which will be probed at 
>> late_initcall() level.
>> (like ./drivers/net/ethernet/ti/cpsw.c => late_initcall(cpsw_init);
> 
> Yes, cpsw_init() should _not_ be a late_initcall.  This is yet another
> example of playing games with ordering probes that we have been trying
> to eliminate.

yes, we're trying to solve such issues and have all TI's drivers initialized
from module_init() level, but as usual this process is not so fast. 

You know, some times ago there was no other way to solve boot ordering issues,
but only to play with init levels :) And, as result, right now in drivers/
and sound/ folders there are >77 occurrences of late_initcall(). 

> 
> Thanks for pointing out one of the resulting problems this causes for the
> deferred probe mechanism.
> 
>> Yes - they probably need to be updated to use module_init(), but that's what
>> we have now). Those drivers will re-trigger deferred device probing if their
>> probe succeeded.
> 
> Yes, if cpsw_init() leads to a successful probe, then deferred device probing
> will be re-triggered.  I do not know if cpsw_init() will be called before or
> after deferred_probe_initcall().  The general initcall mechanism does not
> provide any ordering guarantees between the two functions because they are
> at the same initcall level.

It will be called after and it will re-triggered deferred device probing.
Now ordering of init calls will be specified by drivers/Makefile
which itself is funny thing.

> 
>>
>> As result, it is impossible to say when will it happen the
>> "final round of deferred device probing" :( and final list of drivers which
>> was "deferred forever" will be know only when kernel exits to User space
>> ("deferred forever" - before loading modules).
>>
>> May be, we also can consider adding debug_fs entry which can be used to 
>> display
>> actual state of deferred_probe_pending_list?
>>

 If we have any devices still deferring after _this_ round, that must
 indicate that some resource they want is not available, and that
 should be warned about.

 Of course, modules can defer too - and I made some suggestions in my
 waffle above the patch about that.

>>>
>>> < adding back trimmed, for fuller context >
>>>
>>  /* Sort as many dependencies as possible before exiting 
>> initcalls */
>>  flush_workqueue(deferred_wq);
>> +
>> +/* Now one final round, reporting any devices that remain 
>> deferred */
>> +driver_deferred_probe_report = true;
>> +driver_deferred_probe_trigger();
>> +/* Sort as many dependencies as possible before exiting 
>> initcalls */
>> +flush_workqueue(deferred_wq);
>> +
>>  return 0;
>>}
>>
>>
> 


-- 
regards,
-grygorii
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: linux-next: Tree for Oct 21 (drivers/i2c/busses/i2c-designware-platdrv.c)

[Randy Dunlap]

On 10/20/15 23:16, Stephen Rothwell wrote:
> Hi all,
> 
> There will be no linux-next releases after tomorrow until Nov 2 (kernel
> summit).
> 
> Changes since 20151020:
> 

on i386 or x86_64:

when CONFIG_PM_SLEEP is not enabled:

../drivers/i2c/busses/i2c-designware-platdrv.c:340:13: error: 
'dw_i2c_plat_prepare' undeclared here (not in a function)
  .prepare = dw_i2c_plat_prepare,
 ^
../drivers/i2c/busses/i2c-designware-platdrv.c:341:14: error: 
'dw_i2c_plat_complete' undeclared here (not in a function)
  .complete = dw_i2c_plat_complete,
  ^




-- 
~Randy
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[MinnowBoard] Linux x86 I2C device probing with ACPI

[Christophe Ricard]

From: Christophe Ricard 

Hi,

I am trying to probe slave i2c devices with ACPI running Ubuntu 15.04 and 
kernel 4.3
without success so far.

I've followed guidance here:
http://elinux.org/Minnowboard:MinnowMaxLinuxKernel 


but i found no i2c device are probed at boot.

For example, one default device available in the acpi table is never detected: 
RTEK node (with ID 10EC5640).

When adding my own device to I2C7, my device is never detected as well .

For example the complete modified I2C7 is at the end of the message.

I am compiling the kernel with options:
CONFIG_ACPI_CUSTOM_DSDT_FILE="dsdt.hex"
CONFIG_ACPI_CUSTOM_DSDT=y
CONFIG_ACPI_INITRD_TABLE_OVERRIDE=y

I am running on Minnowboard firmware 0.83 with default options.

Best Regards
Christophe

Device (I2C7)
{
Name (_ADR, Zero)  // _ADR: Address
Name (_HID, "80860F41" /* Intel Baytrail I2C Host Controller */)  
// _HID: Hardware ID
Name (_CID, "80860F41" /* Intel Baytrail I2C Host Controller */)  
// _CID: Compatible ID
Name (_DDN, "Intel(R) I2C Controller #7 - 80860F47") // _DDN: DOS 
Device Name
Name (_UID, 0x07)  // _UID: Unique ID
Name (_DEP, Package (One)  // _DEP: Dependencies
{
PEPD
})
Name (RBUF, ResourceTemplate ()
{
Memory32Fixed (ReadWrite,
0x, // Address Base
0x1000, // Address Length
_Y1F)
Interrupt (ResourceConsumer, Level, ActiveLow, Exclusive, ,, )
{
0x0026,
}
FixedDMA (0x001C, 0x0004, Width32bit, )
FixedDMA (0x001D, 0x0005, Width32bit, )
})
Method (SSCN, 0, NotSerialized)
{
Name (PKG, Package (0x03)
{
0x0200,
0x0200,
0x06
})
Return (PKG) /* \_SB_.I2C7.SSCN.PKG_ */
}

Method (FMCN, 0, NotSerialized)
{
Name (PKG, Package (0x03)
{
0x55,
0x99,
0x06
})
Return (PKG) /* \_SB_.I2C7.FMCN.PKG_ */
}

Method (FPCN, 0, NotSerialized)
{
Name (PKG, Package (0x03)
{
0x1B,
0x3A,
0x06
})
Return (PKG) /* \_SB_.I2C7.FPCN.PKG_ */
}

Method (_HRV, 0, NotSerialized)  // _HRV: Hardware Revision
{
Return (SOCS) /* \SOCS */
}

Method (_CRS, 0, NotSerialized)  // _CRS: Current Resource Settings
{
CreateDWordField (RBUF, \_SB.I2C7._Y1F._BAS, B0BA)  // _BAS: 
Base Address
CreateDWordField (RBUF, \_SB.I2C7._Y1F._LEN, B0LN)  // _LEN: 
Length
B0BA = I70A /* \I70A */
B0LN = I70L /* \I70L */
Return (RBUF) /* \_SB_.I2C7.RBUF */
}

Method (_STA, 0, NotSerialized)  // _STA: Status
{
If ((PCIM == One))
{
Return (Zero)
}

If (((I70A == Zero) || (L27D == One)))
{
Return (Zero)
}

Return (0x0F)
}

Method (_PS3, 0, NotSerialized)  // _PS3: Power State 3
{
PSAT |= 0x03
PSAT |= Zero
}

Method (_PS0, 0, NotSerialized)  // _PS0: Power State 0
{
PSAT &= 0xFFFC
PSAT |= Zero
}

OperationRegion (KEYS, SystemMemory, I71A, 0x0100)
Field (KEYS, DWordAcc, NoLock, WriteAsZeros)
{
Offset (0x84),
PSAT,   32
}

Device (NFC1)
{
Name (_ADR, Zero)  // _ADR: Address
Name (_HID, "NXP5441")  // _HID: Hardware ID
Name (_CID, "NXP5441")  // _CID: Compatible ID
Name (_DDN, "NXP NFC")  // _DDN: DOS Device Name
Name (_UID, One)  // _UID: Unique ID
Method (_CRS, 0, NotSerialized)  // _CRS: Current Resource 
Settings
{
Name (SBUF, ResourceTemplate ()
{
I2cSerialBus (0x0028, ControllerInitiated, 0x00061A80,
AddressingMode7Bit, "\\_SB.I2C7",
0x00, ResourceConsumer, ,
)
Interrupt (ResourceConsumer, Level, ActiveLow, 

Re: [GIT PULL] On-demand device probing

[Mark Brown]

On Wed, Oct 21, 2015 at 11:18:08AM -0700, Frank Rowand wrote:
> On 10/21/2015 9:27 AM, Mark Brown wrote:

> > Overall boot time and time to get some individual built in component up
> > and running aren't the same thing - what this'll do is get things up
> > more in the link order of the leaf consumers rather than deferring those
> > leaf consumers when their dependencies aren't ready yet.

> Thanks!  I read too much into what was being improved.

> So this patch series, which on other merits may be a good idea, is as
> a by product solving a specific ordering issue, moving successful panel
> initialization to an earlier point in the boot sequence, if I now
> understand more correctly.

Yeah, that's my understanding.

> In that context, this seems like yet another ad hoc way of causing the
> probe order to change in a way to solves one specific issue?  Could
> it just as likely move the boot order of some other driver on some
> other board later, to the detriment of somebody else?

Indeed.  My general feeling is that it does make the link order stuff
more predictable and easier to work with and it does have other merits
(in terms of the error reporting, though there's other ways to address
that like the one Russell is proposing).


signature.asc
Description: PGP signature

Re: Alternative approach to solve the deferred probe

[Russell King - ARM Linux]

On Wed, Oct 21, 2015 at 08:36:23AM -0700, Frank Rowand wrote:
> On 10/21/2015 1:18 AM, Russell King - ARM Linux wrote:
> > On Tue, Oct 20, 2015 at 08:58:19PM -0700, Frank Rowand wrote:
> >> On 10/20/2015 8:46 AM, Russell King - ARM Linux wrote:
> 
> < snip >
> 
> >>> +
> >>>  static bool driver_deferred_probe_enable = false;
> >>> +
> >>>  /**
> >>>   * driver_deferred_probe_trigger() - Kick off re-probing deferred devices
> >>>   *
> >>> @@ -188,6 +210,13 @@ static int deferred_probe_initcall(void)
> >>>   driver_deferred_probe_trigger();
> >>
> >> Couldn't you put the "driver_deferred_probe_report = true" here?  And then
> >> not add another round of probes.
> > 
> > The idea is not to report anything for drivers that were deferred
> > during the normal bootup.  The above is part of the normal bootup,
> > and the deferred activity should not be warned about.
> 
> The above is currently the last point for probe to succeed or defer
> (until possibly, as you mentioned, module loading resolves the defer).
> If a probe defers above, it will defer again below.  The set of defers
> should be exactly the same above and below.

Why should it?  Isn't this late_initcall() the first opportunity that
deferred devices get to be re-probed from their first set of attempts
via the drivers having their initcalls called?

If what you're saying is true, what's the point of this late_initcall()?



-- 
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [GIT PULL] On-demand device probing

[Rob Herring]

On Wed, Oct 21, 2015 at 1:18 PM, Frank Rowand  wrote:
> On 10/21/2015 9:27 AM, Mark Brown wrote:
>> On Wed, Oct 21, 2015 at 08:59:51AM -0700, Frank Rowand wrote:
>>> On 10/19/2015 5:34 AM, Tomeu Vizoso wrote:
>>
 To be clear, I was saying that this series should NOT affect total
 boot times much.
>>
>>> I'm confused.  If I understood correctly, improving boot time was
>>> the key justification for accepting this patch set.  For example,
>>> from "[PATCH v7 0/20] On-demand device probing":
>>>
>>>I have a problem with the panel on my Tegra Chromebook taking longer
>>>than expected to be ready during boot (Stéphane Marchesin reported what
>>>is basically the same issue in [0]), and have looked into ordered
>>>probing as a better way of solving this than moving nodes around in the
>>>DT or playing with initcall levels and linking order.
>>>
>>>...
>>>
>>>With this series I get the kernel to output to the panel in 0.5s,
>>>instead of 2.8s.
>>
>> Overall boot time and time to get some individual built in component up
>> and running aren't the same thing - what this'll do is get things up
>> more in the link order of the leaf consumers rather than deferring those
>> leaf consumers when their dependencies aren't ready yet.
>
> Thanks!  I read too much into what was being improved.
>
> So this patch series, which on other merits may be a good idea, is as
> a by product solving a specific ordering issue, moving successful panel
> initialization to an earlier point in the boot sequence, if I now
> understand more correctly.
>
> In that context, this seems like yet another ad hoc way of causing the
> probe order to change in a way to solves one specific issue?  Could
> it just as likely move the boot order of some other driver on some
> other board later, to the detriment of somebody else?

Time to display on is important for many products. Having the console
up as early as possible is another case. CAN bus is another. This is a
real problem that is not just bad drivers.

I don't think it is completely ad hoc. Given all devices are
registered after drivers, drivers will still probe first in initcall
level order and then link order AFAIK. We may not take (more) initcall
level tweak hacks, but that is a much more simple change for
downstream. Don't get me wrong, I'd really like to see a way to
control order independent of initcall level.

Rob
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [GIT PULL] On-demand device probing

[Frank Rowand]

On 10/21/2015 2:12 PM, Rob Herring wrote:
> On Wed, Oct 21, 2015 at 1:18 PM, Frank Rowand  wrote:
>> On 10/21/2015 9:27 AM, Mark Brown wrote:
>>> On Wed, Oct 21, 2015 at 08:59:51AM -0700, Frank Rowand wrote:
 On 10/19/2015 5:34 AM, Tomeu Vizoso wrote:
>>>
> To be clear, I was saying that this series should NOT affect total
> boot times much.
>>>
 I'm confused.  If I understood correctly, improving boot time was
 the key justification for accepting this patch set.  For example,
 from "[PATCH v7 0/20] On-demand device probing":

I have a problem with the panel on my Tegra Chromebook taking longer
than expected to be ready during boot (Stéphane Marchesin reported what
is basically the same issue in [0]), and have looked into ordered
probing as a better way of solving this than moving nodes around in the
DT or playing with initcall levels and linking order.

...

With this series I get the kernel to output to the panel in 0.5s,
instead of 2.8s.
>>>
>>> Overall boot time and time to get some individual built in component up
>>> and running aren't the same thing - what this'll do is get things up
>>> more in the link order of the leaf consumers rather than deferring those
>>> leaf consumers when their dependencies aren't ready yet.
>>
>> Thanks!  I read too much into what was being improved.
>>
>> So this patch series, which on other merits may be a good idea, is as
>> a by product solving a specific ordering issue, moving successful panel
>> initialization to an earlier point in the boot sequence, if I now
>> understand more correctly.
>>
>> In that context, this seems like yet another ad hoc way of causing the
>> probe order to change in a way to solves one specific issue?  Could
>> it just as likely move the boot order of some other driver on some
>> other board later, to the detriment of somebody else?
> 
> Time to display on is important for many products. Having the console
> up as early as possible is another case. CAN bus is another. This is a
> real problem that is not just bad drivers.

Yes, I agree.

What I am seeing is that there continues to be a need for the ability
to explicitly order at least some driver initialization (at some
granularity), despite the push back against explicit ordering that
has been present in the past.


> I don't think it is completely ad hoc. Given all devices are
> registered after drivers, drivers will still probe first in initcall
> level order and then link order AFAIK. We may not take (more) initcall
> level tweak hacks, but that is a much more simple change for
> downstream. Don't get me wrong, I'd really like to see a way to
> control order independent of initcall level.
> 
> Rob

Yep, it is not directly ad hoc, just a fortunate side effect in
this case.  So just accidently ad hoc. :-)

-Frank

--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 1/1] i2c: add ACPI support for I2C mux ports

[Rafael J. Wysocki]

Hi,

On Wed, Oct 21, 2015 at 11:25 AM, Dustin Byford
 wrote:
> On Wed Oct 21 12:08, Mika Westerberg wrote:
>> I don't really have strong feelings whether it should be the I2C core or
>> individual drivers setting the ACPI companion. However, it would be nice
>> to match DT here and they assign their of_node per driver.
>
> OK with me, if we can convince Rafael this is a good idea, I'll send a
> new revision with drivers setting the companion.

If you can guarantee that ACPI PM or anything like _DS or _SRS will
never be invoked for the device objects that "inherit" the ACPI
companion from their parent, it at least is not outright dangerous.

That said I'm thinking that may need some more sophisticated approach
here, so we really can guarantee certain things, but that's for the
future.

Thanks,
Rafael
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 5/9] i2c: rcar: init new messages in irq

[Laurent Pinchart]

Hi Wolfram,

On Thursday 03 September 2015 22:20:09 Wolfram Sang wrote:
> From: Wolfram Sang 
> 
> Setting up new messages was done in process context while handling a
> message was in interrupt context. Because of the HW design, this IP core
> is sensitive to timing, so the context switches were too expensive. Move
> this setup to interrupt context as well.
> 
> In my test setup, this fixed the occasional 'data byte sent twice' issue
> which a number of people have seen. It also fixes to send REP_START
> after a read message which was wrongly send as a STOP + START sequence
> before.

I'm afraid this patch has been found by git bisect to break HDMI on Koelsch 
:-( 

The regmap_read(adv7511->regmap, ADV7511_REG_CHIP_REVISION, ) call in 
drivers/gpu/drm/i2c/adv7511.c returns -ENXIO.

Reverting the patch on top of Geert's current drivers master branch fixes the 
problem.

> Signed-off-by: Wolfram Sang 
> ---
>  drivers/i2c/busses/i2c-rcar.c | 74 +++-
>  1 file changed, 37 insertions(+), 37 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-rcar.c b/drivers/i2c/busses/i2c-rcar.c
> index 6e459a338ccc75..36c79301044b71 100644
> --- a/drivers/i2c/busses/i2c-rcar.c
> +++ b/drivers/i2c/busses/i2c-rcar.c
> @@ -266,6 +266,13 @@ static void rcar_i2c_prepare_msg(struct rcar_i2c_priv
> *priv) rcar_i2c_write(priv, ICMIER, read ? RCAR_IRQ_RECV : RCAR_IRQ_SEND);
> }
> 
> +static void rcar_i2c_next_msg(struct rcar_i2c_priv *priv)
> +{
> + priv->msg++;
> + priv->msgs_left--;
> + rcar_i2c_prepare_msg(priv);
> +}
> +
>  /*
>   *   interrupt functions
>   */
> @@ -308,21 +315,17 @@ static int rcar_i2c_irq_send(struct rcar_i2c_priv
> *priv, u32 msr) * [ICRXTX] -> [SHIFT] -> [I2C bus]
>*/
> 
> - if (priv->flags & ID_LAST_MSG)
> + if (priv->flags & ID_LAST_MSG) {
>   /*
>* If current msg is the _LAST_ msg,
>* prepare stop condition here.
>* ID_DONE will be set on STOP irq.
>*/
>   rcar_i2c_write(priv, ICMCR, RCAR_BUS_PHASE_STOP);
> - else
> - /*
> -  * If current msg is _NOT_ last msg,
> -  * it doesn't call stop phase.
> -  * thus, there is no STOP irq.
> -  * return ID_DONE here.
> -  */
> - return ID_DONE;
> + } else {
> + rcar_i2c_next_msg(priv);
> + return 0;
> + }
>   }
> 
>   rcar_i2c_write(priv, ICMSR, RCAR_IRQ_ACK_SEND);
> @@ -366,7 +369,10 @@ static int rcar_i2c_irq_recv(struct rcar_i2c_priv
> *priv, u32 msr) else
>   rcar_i2c_write(priv, ICMCR, RCAR_BUS_PHASE_DATA);
> 
> - rcar_i2c_write(priv, ICMSR, RCAR_IRQ_ACK_RECV);
> + if (priv->pos == msg->len && !(priv->flags & ID_LAST_MSG))
> + rcar_i2c_next_msg(priv);
> + else
> + rcar_i2c_write(priv, ICMSR, RCAR_IRQ_ACK_RECV);
> 
>   return 0;
>  }
> @@ -461,6 +467,7 @@ static irqreturn_t rcar_i2c_irq(int irq, void *ptr)
> 
>   /* Stop */
>   if (msr & MST) {
> + priv->msgs_left--; /* The last message also made it */
>   rcar_i2c_flags_set(priv, ID_DONE);
>   goto out;
>   }
> @@ -500,35 +507,28 @@ static int rcar_i2c_master_xfer(struct i2c_adapter
> *adap, /* This HW can't send STOP after address phase */
>   if (msgs[i].len == 0) {
>   ret = -EOPNOTSUPP;
> - break;
> - }
> -
> - /* init each data */
> - priv->msg = [i];
> - priv->msgs_left = num - i;
> -
> - rcar_i2c_prepare_msg(priv);
> -
> - time_left = wait_event_timeout(priv->wait,
> -  rcar_i2c_flags_has(priv, ID_DONE),
> -  adap->timeout);
> - if (!time_left) {
> - rcar_i2c_init(priv);
> - ret = -ETIMEDOUT;
> - break;
> - }
> -
> - if (rcar_i2c_flags_has(priv, ID_NACK)) {
> - ret = -ENXIO;
> - break;
> - }
> -
> - if (rcar_i2c_flags_has(priv, ID_ARBLOST)) {
> - ret = -EAGAIN;
> - break;
> + goto out;
>   }
> + }
> 
> - ret = i + 1; /* The number of transfer */
> + /* init data */
> + priv->msg = msgs;
> + priv->msgs_left = num;
> +
> + rcar_i2c_prepare_msg(priv);
> +
> + time_left = wait_event_timeout(priv->wait,
> +  rcar_i2c_flags_has(priv, ID_DONE),
> +  num * 

Re: [GIT PULL] On-demand device probing

[Rafael J. Wysocki]

On Wednesday, October 21, 2015 10:55:14 AM Geert Uytterhoeven wrote:
> Hi Rafael,
> 
> On Wed, Oct 21, 2015 at 1:34 AM, Rafael J. Wysocki  wrote:
> > On Tuesday, October 20, 2015 09:15:01 AM Rob Herring wrote:
> >> On Tue, Oct 20, 2015 at 2:56 AM, Rafael J. Wysocki  
> >> wrote:
> >> > ACPI uses platform devices too.  In fact, ACPI device objects are 
> >> > enumerated as
> >> > platform devices by default now.
> >>
> >> Okay, I should have grepped for that:
> >> drivers/base/platform.c:ACPI_COMPANION_SET(>dev, 
> >> NULL);
> >> drivers/base/platform.c:len = acpi_device_modalias(dev, buf,
> >> PAGE_SIZE -1);
> >> drivers/base/platform.c:rc = acpi_device_uevent_modalias(dev, env);
> >> drivers/base/platform.c:/* Then try ACPI style match */
> >> drivers/base/platform.c:if (acpi_driver_match_device(dev, drv))
> >>
> >> These are all cases which have DT version as well, so we're not really
> >> all that different here. There's a few more for DT, but that probably
> >> means you have just not hit the problems we have yet. For example,
> >> what happens if you have an interrupt line in which the controller is
> >> probed after the device connected to the interrupt line? That required
> >> resolving irqs in platform_get_irq rather than using static resources
> >> to support deferred probe.
> >
> > We don't have this particular problem, because the IRQ controllers are
> > enumerated in a special way.
> 
> What does "in a special way" mean? Can you please be more specific?
> 
> Can you have interrupt controllers that depend on clocks, pin controllers,
> and PM domains?

Currently, there's no native way to represent those dependencies in ACPI.

Thanks,
Rafael

--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] i2c: designware: enable High-speed mode for pcidrv

[Xiang Wang]

2015-10-15 17:40 GMT+08:00 Andy Shevchenko :
> On Thu, 2015-10-15 at 11:32 +0300, Jarkko Nikula wrote:
>> On 10/15/2015 08:46 AM, Xiang Wang wrote:
>> >
>> > In conclusion, we have 2 solutions to set the i2c controller speed
>> > mode (pci driver):
>> > 1) use hardcode value in pci driver
>> > 2) use frequency setting of "i2c device" in ACPI table (more
>> > flexible,
>> > but looks a bit strange)
>> >
>> > Do you have any preference/suggestions for above solutions? Thanks
>>
>> I don't think we can hard code especially the high-speed mode because
>>
>> most typically buses are populated with slower devices.
>>
>> Things are a bit more clear when ACPI provides timing parameters for
>> the
>> bus (for standard and fast speed modes at the moment in
>> i2c-designware-platdrv.c: dw_i2c_acpi_configure()) but still I think
>> the
>> ACPI namespace walk may be needed against potential BIOS
>> misconfigurations. For instance if it provides timing parameters for
>> all
>> speeds but there are devices with lower speed on the same bus.
>>
>> I'd take these timing parameters as configuration data for bus
>> features
>> but actual speed (speed bits in IC_CON register) is defined
>> separately.
>> To me it looks only way to achieve that is to pick slowest device
>> from
>> I2cSerialBus resource descriptors.
>
> Should it (ACPI walk) be done in PCI case as well? If so, then it needs
> to be done up to i2c-core. There you may adjust the bus speed whenever
> slave device is enumerated.
>
I think the "ACPI walk for bus speed" also works for PCI case. It'll
be good to do this in i2c-core.
By doing this we can get a minimum device speed for this bus. I2C bus
drivers can use this speed to set corresponding mode into i2c
controller.
Waiting for comments from others.

> For PCI case you have still to have hardcoded values and it should be
> maximum supported by the bus I think. When you have implemented above
> algorithm you may do this safely. Am I missing something?
Agree. It'll be safer to set a hardcoded maximum supported speed of
the bus for PCI case.
I2C bus driver can use MIN(speed_get_by_ACPI_walk,
hardcoded_max_supported_speed) for bus speed.

>
> --
> Andy Shevchenko 
> Intel Finland Oy



-- 
Regards,
Xiang
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/9] i2c: brcmstb: fix typo in i2c-brcmstb

[Florian Fainelli]

Le 20/10/2015 19:36, Jaedon Shin a écrit :
> Fixes the "definitions" where it is spelled "defintions".
> 
> Signed-off-by: Jaedon Shin 

Acked-by: Florian Fainelli 

> ---
>  drivers/i2c/busses/i2c-brcmstb.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/i2c/busses/i2c-brcmstb.c 
> b/drivers/i2c/busses/i2c-brcmstb.c
> index 8e9637eea512..6b8bbf99880d 100644
> --- a/drivers/i2c/busses/i2c-brcmstb.c
> +++ b/drivers/i2c/busses/i2c-brcmstb.c
> @@ -41,7 +41,7 @@
>  #define BSC_CTL_REG_INT_EN_SHIFT 6
>  #define BSC_CTL_REG_DIV_CLK_MASK 0x0080
>  
> -/* BSC_IIC_ENABLE r/w enable and interrupt field defintions */
> +/* BSC_IIC_ENABLE r/w enable and interrupt field definitions */
>  #define BSC_IIC_EN_RESTART_MASK  0x0040
>  #define BSC_IIC_EN_NOSTART_MASK  0x0020
>  #define BSC_IIC_EN_NOSTOP_MASK   0x0010
> 


-- 
Florian
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/9] i2c: brcmstb: make the driver buildable on BMIPS_GENERIC

[Florian Fainelli]

Le 20/10/2015 19:36, Jaedon Shin a écrit :
> The BCM7xxx ARM and MIPS platforms share a similar hardware block for
> I2C.
> 
> Signed-off-by: Jaedon Shin 

Acked-by: Florian Fainelli 

> ---
>  drivers/i2c/busses/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
> index 08b86178e8fb..fd983c5b36f2 100644
> --- a/drivers/i2c/busses/Kconfig
> +++ b/drivers/i2c/busses/Kconfig
> @@ -394,7 +394,7 @@ config I2C_BCM_KONA
>  
>  config I2C_BRCMSTB
>   tristate "BRCM Settop I2C controller"
> - depends on ARCH_BRCMSTB || COMPILE_TEST
> + depends on ARCH_BRCMSTB || BMIPS_GENERIC || COMPILE_TEST
>   default y
>   help
> If you say yes to this option, support will be included for the
> 


-- 
Florian
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/9] i2c: brcmstb: add missing parenthesis

[Florian Fainelli]

Le 20/10/2015 19:36, Jaedon Shin a écrit :
> Add the necessary parenthesis for NOACK condition.
> 
> Signed-off-by: Jaedon Shin 

Acked-by: Florian Fainelli 

> ---
>  drivers/i2c/busses/i2c-brcmstb.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/i2c/busses/i2c-brcmstb.c 
> b/drivers/i2c/busses/i2c-brcmstb.c
> index 6b8bbf99880d..2d7d155029dc 100644
> --- a/drivers/i2c/busses/i2c-brcmstb.c
> +++ b/drivers/i2c/busses/i2c-brcmstb.c
> @@ -305,7 +305,7 @@ static int brcmstb_send_i2c_cmd(struct brcmstb_i2c_dev 
> *dev,
>   }
>  
>   if ((CMD_RD || CMD_WR) &&
> - bsc_readl(dev, iic_enable) & BSC_IIC_EN_NOACK_MASK) {
> + (bsc_readl(dev, iic_enable) & BSC_IIC_EN_NOACK_MASK)) {
>   rc = -EREMOTEIO;
>   dev_dbg(dev->device, "controller received NOACK intr for %s\n",
>   cmd_string[cmd]);
> 


-- 
Florian
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 8/9] MIPS: BMIPS: brcmstb: add I2C node for bcm7360

[Florian Fainelli]

Le 20/10/2015 19:37, Jaedon Shin a écrit :
> Add I2C device nodes to BMIPS based BCM7360 platform.
> 
> Signed-off-by: Jaedon Shin 

Acked-by: Florian Fainelli 

> ---
>  arch/mips/boot/dts/brcm/bcm7360.dtsi | 62 
> ++--
>  arch/mips/boot/dts/brcm/bcm97360svmb.dts | 16 +
>  2 files changed, 76 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/mips/boot/dts/brcm/bcm7360.dtsi 
> b/arch/mips/boot/dts/brcm/bcm7360.dtsi
> index 9e1e571ba346..7e5f76040fb8 100644
> --- a/arch/mips/boot/dts/brcm/bcm7360.dtsi
> +++ b/arch/mips/boot/dts/brcm/bcm7360.dtsi
> @@ -81,14 +81,32 @@
>   compatible = "brcm,bcm7120-l2-intc";
>   reg = <0x406600 0x8>;
>  
> - brcm,int-map-mask = <0x44>;
> + brcm,int-map-mask = <0x44>, <0x700>;
>   brcm,int-fwd-mask = <0x7>;
>  
>   interrupt-controller;
>   #interrupt-cells = <1>;
>  
>   interrupt-parent = <_intc>;
> - interrupts = <56>;
> + interrupts = <56>, <54>;
> + interrupt-names = "upg_main", "upg_bsc";
> + };
> +
> + upg_aon_irq0_intc: upg_aon_irq0_intc@408b80 {
> + compatible = "brcm,bcm7120-l2-intc";
> + reg = <0x408b80 0x8>;
> +
> + brcm,int-map-mask = <0x40>, <0x800>, <0x10>;
> + brcm,int-fwd-mask = <0>;
> + brcm,irq-can-wake;
> +
> + interrupt-controller;
> + #interrupt-cells = <1>;
> +
> + interrupt-parent = <_intc>;
> + interrupts = <57>, <55>, <59>;
> + interrupt-names = "upg_main_aon", "upg_bsc_aon",
> +   "upg_spi";
>   };
>  
>   sun_top_ctrl: syscon@404000 {
> @@ -138,6 +156,46 @@
>   status = "disabled";
>   };
>  
> + bsca: i2c@406200 {
> +   clock-frequency = <39>;
> +   compatible = "brcm,brcmstb-i2c";
> +   interrupt-parent = <_irq0_intc>;
> +   reg = <0x406200 0x58>;
> +   interrupts = <24>;
> +   interrupt-names = "upg_bsca";
> +   status = "disabled";
> + };
> +
> + bscb: i2c@406280 {
> +   clock-frequency = <39>;
> +   compatible = "brcm,brcmstb-i2c";
> +   interrupt-parent = <_irq0_intc>;
> +   reg = <0x406280 0x58>;
> +   interrupts = <25>;
> +   interrupt-names = "upg_bscb";
> +   status = "disabled";
> + };
> +
> + bscc: i2c@406300 {
> +   clock-frequency = <39>;
> +   compatible = "brcm,brcmstb-i2c";
> +   interrupt-parent = <_irq0_intc>;
> +   reg = <0x406300 0x58>;
> +   interrupts = <26>;
> +   interrupt-names = "upg_bscc";
> +   status = "disabled";
> + };
> +
> + bscd: i2c@408980 {
> +   clock-frequency = <39>;
> +   compatible = "brcm,brcmstb-i2c";
> +   interrupt-parent = <_aon_irq0_intc>;
> +   reg = <0x408980 0x58>;
> +   interrupts = <27>;
> +   interrupt-names = "upg_bscd";
> +   status = "disabled";
> + };
> +
>   enet0: ethernet@43 {
>   phy-mode = "internal";
>   phy-handle = <>;
> diff --git a/arch/mips/boot/dts/brcm/bcm97360svmb.dts 
> b/arch/mips/boot/dts/brcm/bcm97360svmb.dts
> index eee8b0e32681..d48462e091f1 100644
> --- a/arch/mips/boot/dts/brcm/bcm97360svmb.dts
> +++ b/arch/mips/boot/dts/brcm/bcm97360svmb.dts
> @@ -29,6 +29,22 @@
>   status = "okay";
>  };
>  
> + {
> + status = "okay";
> +};
> +
> + {
> + status = "okay";
> +};
> +
> + {
> + status = "okay";
> +};
> +
> + {
> + status = "okay";
> +};
> +
>   {
>   status = "okay";
>  };
> 


-- 
Florian
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 6/9] MIPS: BMIPS: brcmstb: add I2C node for bcm7346

[Florian Fainelli]

Le 20/10/2015 19:36, Jaedon Shin a écrit :
> Add I2C device nodes to BMIPS based BCM7346 platform.
> 
> Signed-off-by: Jaedon Shin 

Acked-by: Florian Fainelli 
-- 
Florian
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 9/9] MIPS: BMIPS: brcmstb: add I2C node for bcm7362

[Florian Fainelli]

Le 20/10/2015 19:37, Jaedon Shin a écrit :
> Add I2C device nodes to BMIPS based BCM7362 platform.
> 
> Signed-off-by: Jaedon Shin 

Acked-by: Florian Fainelli 

> ---
>  arch/mips/boot/dts/brcm/bcm7362.dtsi | 52 
> ++--
>  arch/mips/boot/dts/brcm/bcm97362svmb.dts | 12 
>  2 files changed, 62 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/mips/boot/dts/brcm/bcm7362.dtsi 
> b/arch/mips/boot/dts/brcm/bcm7362.dtsi
> index 6e65db86fc61..5f817be2553c 100644
> --- a/arch/mips/boot/dts/brcm/bcm7362.dtsi
> +++ b/arch/mips/boot/dts/brcm/bcm7362.dtsi
> @@ -87,14 +87,32 @@
>   compatible = "brcm,bcm7120-l2-intc";
>   reg = <0x406600 0x8>;
>  
> - brcm,int-map-mask = <0x44>;
> + brcm,int-map-mask = <0x44>, <0x700>;
>   brcm,int-fwd-mask = <0x7>;
>  
>   interrupt-controller;
>   #interrupt-cells = <1>;
>  
>   interrupt-parent = <_intc>;
> - interrupts = <56>;
> + interrupts = <56>, <54>;
> + interrupt-names = "upg_main", "upg_bsc";
> + };
> +
> + upg_aon_irq0_intc: upg_aon_irq0_intc@408b80 {
> + compatible = "brcm,bcm7120-l2-intc";
> + reg = <0x408b80 0x8>;
> +
> + brcm,int-map-mask = <0x40>, <0x800>, <0x10>;
> + brcm,int-fwd-mask = <0>;
> + brcm,irq-can-wake;
> +
> + interrupt-controller;
> + #interrupt-cells = <1>;
> +
> + interrupt-parent = <_intc>;
> + interrupts = <57>, <55>, <59>;
> + interrupt-names = "upg_main_aon", "upg_bsc_aon",
> +   "upg_spi";
>   };
>  
>   sun_top_ctrl: syscon@404000 {
> @@ -144,6 +162,36 @@
>   status = "disabled";
>   };
>  
> + bsca: i2c@406200 {
> +   clock-frequency = <39>;
> +   compatible = "brcm,brcmstb-i2c";
> +   interrupt-parent = <_irq0_intc>;
> +   reg = <0x406200 0x58>;
> +   interrupts = <24>;
> +   interrupt-names = "upg_bsca";
> +   status = "disabled";
> + };
> +
> + bscb: i2c@406280 {
> +   clock-frequency = <39>;
> +   compatible = "brcm,brcmstb-i2c";
> +   interrupt-parent = <_irq0_intc>;
> +   reg = <0x406280 0x58>;
> +   interrupts = <25>;
> +   interrupt-names = "upg_bscb";
> +   status = "disabled";
> + };
> +
> + bscd: i2c@408980 {
> +   clock-frequency = <39>;
> +   compatible = "brcm,brcmstb-i2c";
> +   interrupt-parent = <_aon_irq0_intc>;
> +   reg = <0x408980 0x58>;
> +   interrupts = <27>;
> +   interrupt-names = "upg_bscd";
> +   status = "disabled";
> + };
> +
>   enet0: ethernet@43 {
>   phy-mode = "internal";
>   phy-handle = <>;
> diff --git a/arch/mips/boot/dts/brcm/bcm97362svmb.dts 
> b/arch/mips/boot/dts/brcm/bcm97362svmb.dts
> index 739c2ef5663b..9c99bfd1e781 100644
> --- a/arch/mips/boot/dts/brcm/bcm97362svmb.dts
> +++ b/arch/mips/boot/dts/brcm/bcm97362svmb.dts
> @@ -29,6 +29,18 @@
>   status = "okay";
>  };
>  
> + {
> + status = "okay";
> +};
> +
> + {
> + status = "okay";
> +};
> +
> + {
> + status = "okay";
> +};
> +
>   {
>   status = "okay";
>  };
> 


-- 
Florian
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 7/9] MIPS: BMIPS: brcmstb: add I2C node for bcm7358

[Florian Fainelli]

Le 20/10/2015 19:36, Jaedon Shin a écrit :
> Add I2C device nodes to BMIPS based BCM7358 platform.
> 
> Signed-off-by: Jaedon Shin 

Acked-by: Florian Fainelli 

> ---
>  arch/mips/boot/dts/brcm/bcm7358.dtsi | 62 
> ++--
>  arch/mips/boot/dts/brcm/bcm97358svmb.dts | 16 +
>  2 files changed, 76 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/mips/boot/dts/brcm/bcm7358.dtsi 
> b/arch/mips/boot/dts/brcm/bcm7358.dtsi
> index 277a90adc1a7..8e2501694d03 100644
> --- a/arch/mips/boot/dts/brcm/bcm7358.dtsi
> +++ b/arch/mips/boot/dts/brcm/bcm7358.dtsi
> @@ -81,14 +81,32 @@
>   compatible = "brcm,bcm7120-l2-intc";
>   reg = <0x406600 0x8>;
>  
> - brcm,int-map-mask = <0x44>;
> + brcm,int-map-mask = <0x44>, <0x700>;
>   brcm,int-fwd-mask = <0x7>;
>  
>   interrupt-controller;
>   #interrupt-cells = <1>;
>  
>   interrupt-parent = <_intc>;
> - interrupts = <56>;
> + interrupts = <56>, <54>;
> + interrupt-names = "upg_main", "upg_bsc";
> + };
> +
> + upg_aon_irq0_intc: upg_aon_irq0_intc@408b80 {
> + compatible = "brcm,bcm7120-l2-intc";
> + reg = <0x408b80 0x8>;
> +
> + brcm,int-map-mask = <0x40>, <0x800>, <0x10>;
> + brcm,int-fwd-mask = <0>;
> + brcm,irq-can-wake;
> +
> + interrupt-controller;
> + #interrupt-cells = <1>;
> +
> + interrupt-parent = <_intc>;
> + interrupts = <57>, <55>, <59>;
> + interrupt-names = "upg_main_aon", "upg_bsc_aon",
> +   "upg_spi";
>   };
>  
>   sun_top_ctrl: syscon@404000 {
> @@ -138,6 +156,46 @@
>   status = "disabled";
>   };
>  
> + bsca: i2c@406200 {
> +   clock-frequency = <39>;
> +   compatible = "brcm,brcmstb-i2c";
> +   interrupt-parent = <_irq0_intc>;
> +   reg = <0x406200 0x58>;
> +   interrupts = <24>;
> +   interrupt-names = "upg_bsca";
> +   status = "disabled";
> + };
> +
> + bscb: i2c@406280 {
> +   clock-frequency = <39>;
> +   compatible = "brcm,brcmstb-i2c";
> +   interrupt-parent = <_irq0_intc>;
> +   reg = <0x406280 0x58>;
> +   interrupts = <25>;
> +   interrupt-names = "upg_bscb";
> +   status = "disabled";
> + };
> +
> + bscc: i2c@406300 {
> +   clock-frequency = <39>;
> +   compatible = "brcm,brcmstb-i2c";
> +   interrupt-parent = <_irq0_intc>;
> +   reg = <0x406300 0x58>;
> +   interrupts = <26>;
> +   interrupt-names = "upg_bscc";
> +   status = "disabled";
> + };
> +
> + bscd: i2c@408980 {
> +   clock-frequency = <39>;
> +   compatible = "brcm,brcmstb-i2c";
> +   interrupt-parent = <_aon_irq0_intc>;
> +   reg = <0x408980 0x58>;
> +   interrupts = <27>;
> +   interrupt-names = "upg_bscd";
> +   status = "disabled";
> + };
> +
>   enet0: ethernet@43 {
>   phy-mode = "internal";
>   phy-handle = <>;
> diff --git a/arch/mips/boot/dts/brcm/bcm97358svmb.dts 
> b/arch/mips/boot/dts/brcm/bcm97358svmb.dts
> index a8dc01e30313..02ce6b429dc4 100644
> --- a/arch/mips/boot/dts/brcm/bcm97358svmb.dts
> +++ b/arch/mips/boot/dts/brcm/bcm97358svmb.dts
> @@ -29,6 +29,22 @@
>   status = "okay";
>  };
>  
> + {
> + status = "okay";
> +};
> +
> + {
> + status = "okay";
> +};
> +
> + {
> + status = "okay";
> +};
> +
> + {
> + status = "okay";
> +};
> +
>   {
>   status = "okay";
>  };
> 


-- 
Florian
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Alternative approach to solve the deferred probe

[Grygorii Strashko]

Hi Russell,

On 10/21/2015 08:20 PM, Russell King - ARM Linux wrote:
> On Wed, Oct 21, 2015 at 07:55:29PM +0300, Grygorii Strashko wrote:
>> On 10/21/2015 06:36 PM, Frank Rowand wrote:
>>> The above is currently the last point for probe to succeed or defer
>>> (until possibly, as you mentioned, module loading resolves the defer).
>>> If a probe defers above, it will defer again below.  The set of defers
>>> should be exactly the same above and below.
>>>
>>
>> Unfortunately this is not "the last point for probe to succeed or defer".
> 
> Of course it isn't.  Being pedantic, there's actually no such thing,
> because the point that the kernel as finished booting can never actually
> be determined with things like modules being present.  That's something
> I've acknowledged from the start of this.
> 
>> There are still a bunch of drivers in Kernel which will be probed at 
>> late_initcall() level.
>> (like ./drivers/net/ethernet/ti/cpsw.c => late_initcall(cpsw_init);
>> Yes - they probably need to be updated to use module_init(), but that's what
>> we have now). Those drivers will re-trigger deferred device probing if their
>> probe succeeded.
> 
> Maybe this particular late_initcall() which triggers off the deferred
> probing should be moved to its own really_late_initcall() which happens
> as the very last thing - I think this is intended to run after everything
> else has had a chance to probe once.
> 
>> As result, it is impossible to say when will it happen the
>> "final round of deferred device probing" :( and final list of drivers
>> which was "deferred forever" will be know only when kernel exits to
>> User space  ("deferred forever" - before loading modules).
>>
>> May be, we also can consider adding debug_fs entry which can be used to
>> display actual state of deferred_probe_pending_list?
> 
> There are complaints in this thread about the existing deferred probing
> implementation being hard to debug - where it's known that a device
> has deferred, but it's not known why that happened.
> 
> That would be solved by my proposal, as this final round of probing
> before entering userspace after _all_ normal device probes have been
> attempted once and then we've tried to satisfy the deferred probe
> (okay, that's what it's _supposed_ to be - and as it takes three lines
> to write it, you'll excuse me if I just use the abbreviated "final
> round of deferred probe" which is much shorter - but remember that
> the long version is what I actually mean) would produce a list of
> not only the devices that failed to probe, but also the cause of the
> deferred probes.
> 
> My proposal would ensure that subsystems are happier to add these
> prints, because in the normal scenario where we have deferred probing,
> we're not littering the console log with lots of useless failure
> messages which make people stop and think "now did device X probe?"
> It also means scripts in our boot farms can more effectively analyse
> the log and determine whether the boot was actually successful and
> contained no errors.
> 
> Merely printing the list of devices which have been deferred is next
> to useless.  The next question will always be "why did device X defer?"
> and if that can't be answered, it means people having to spend a long
> time adding lots of printks to the kernel at lots of -EPROBE_DEFER
> returning sites or in the relevant drivers, tracing through the code
> back towards the -EPROBE_DEFER sites to try and track it down.
> 

I perfectly understand your proposal and spent a lot of time trying to
debug such kind issues also (and using printks).  

But I worry a bit (and that my main point) about these few additional
rounds of deferred device probing which I have right now and which allows
some of drivers to finish, finally, their probes successfully.
With proposed change I'll get more messages in boot log, but some of
them will belong to drivers which have been probed successfully and so,
they will be not really useful.

As result, I think, the most important thing is to identify (or create)
some point during kernel boot when it will be possible to say that all
built-in drivers (at least) finish their probes 100% (done or defer).

Might be do_initcalls() can be updated (smth like this):
static void __init do_initcalls(void)
{
int level;

for (level = 0; level < ARRAY_SIZE(initcall_levels) - 1; level++)
do_initcall_level(level);

+   wait_for_device_probe();
+   /* Now one final round, reporting any devices that remain deferred */
+   driver_deferred_probe_report = true;
+   driver_deferred_probe_trigger();
+   wait_for_device_probe();
}

Also, in my opinion, it will be useful if this debugging feature will be 
optional.

Thanks.
-- 
regards,
-grygorii
S/ILKP
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Alternative approach to solve the deferred probe

[Frank Rowand]

On 10/21/2015 1:35 PM, Russell King - ARM Linux wrote:
> On Wed, Oct 21, 2015 at 08:36:23AM -0700, Frank Rowand wrote:
>> On 10/21/2015 1:18 AM, Russell King - ARM Linux wrote:
>>> On Tue, Oct 20, 2015 at 08:58:19PM -0700, Frank Rowand wrote:
 On 10/20/2015 8:46 AM, Russell King - ARM Linux wrote:
>>
>> < snip >
>>
> +
>  static bool driver_deferred_probe_enable = false;
> +
>  /**
>   * driver_deferred_probe_trigger() - Kick off re-probing deferred devices
>   *
> @@ -188,6 +210,13 @@ static int deferred_probe_initcall(void)
>   driver_deferred_probe_trigger();

 Couldn't you put the "driver_deferred_probe_report = true" here?  And then
 not add another round of probes.
>>>
>>> The idea is not to report anything for drivers that were deferred
>>> during the normal bootup.  The above is part of the normal bootup,
>>> and the deferred activity should not be warned about.
>>
>> The above is currently the last point for probe to succeed or defer
>> (until possibly, as you mentioned, module loading resolves the defer).
>> If a probe defers above, it will defer again below.  The set of defers
>> should be exactly the same above and below.
> 
> Why should it?

My assertion was incorrect.  A probe in the deferral processing can
result in the driver being placed on the new deferred list, then when
a later probe of another deferred driver succeeds, the first driver
will be moved to the active deferred list, and might succeed on
the second probe attempt (or with the current messages would result
in a second set of deferred messages).  So yes, placing
"driver_deferred_probe_report = true" where your patch put it and
running through the deferred probe processing again is correct.

-Frank
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [GIT PULL] On-demand device probing

[Rafael J. Wysocki]

On Tuesday, October 20, 2015 06:21:55 PM Tomeu Vizoso wrote:
> On 20 October 2015 at 18:04, Alan Stern  wrote:
> > On Tue, 20 Oct 2015, Mark Brown wrote:
> >
> >> On Tue, Oct 20, 2015 at 10:40:03AM -0400, Alan Stern wrote:
> >>
> >> > Furthermore, that applies only to devices that use synchronous suspend.
> >> > Async suspend is becoming common, and there the only restrictions are
> >> > parent-child relations plus whatever explicit requirements that drivers
> >> > impose by calling device_pm_wait_for_dev().
> >>
> >> Hrm, this is the first I'd noticed that feature though I see the initial
> >> commit dates from January.
> >
> > Async suspend and device_pm_wait_for_dev() were added in January 2010,
> > not 2015!
> >
> >>  It looks like most of the users are PCs at
> >> the minute but we should be using it more widely for embedded things,
> >> there's definitely some cases I'm aware of where it will allow us to
> >> remove some open coding.
> >>
> >> It does seem like we want to be feeding dependency information we
> >> discover for probing way into the suspend dependencies...
> >
> > Rafael has been thinking about a way to do this systematically.
> > Nothing concrete has emerged yet.
> 
> This iteration of the series would make this quite easy, as
> dependencies are calculated before probes are attempted:
> 
> https://lkml.org/lkml/2015/6/17/311

Well, if you know how to represent "links" between devices, the mechanism
introduced here doesn't really add much value, because in that case the
core knows what the dependencies are in the first place and can only
defer the probes that have to be deferred.

Thanks,
Rafael

--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] i2c: designware: Fix build error when !CONFIG_PM_SLEEP

[Jarkko Nikula]

Commit 6ad6fde3970c ("i2c: designware: Rename platform driver probe and PM
functions") introduced "'dw_i2c_plat_prepare' undeclared here" and
"'dw_i2c_plat_complete' undeclared here" build errors when CONFIG_PM_SLEEP
is not set.

Fix this by renaming NULL defined dw_i2c_prepare and dw_i2c_complete PM
hooks to dw_i2c_plat_prepare and dw_i2c_plat_complete since this was
obviously missing from the commit.

Reported-by: kbuild test robot 
Signed-off-by: Jarkko Nikula 
---
 drivers/i2c/busses/i2c-designware-platdrv.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c 
b/drivers/i2c/busses/i2c-designware-platdrv.c
index f6b252a8ffd1..eb55760ccd9f 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -307,8 +307,8 @@ static void dw_i2c_plat_complete(struct device *dev)
pm_request_resume(dev);
 }
 #else
-#define dw_i2c_prepare NULL
-#define dw_i2c_completeNULL
+#define dw_i2c_plat_prepareNULL
+#define dw_i2c_plat_complete   NULL
 #endif
 
 #ifdef CONFIG_PM
-- 
2.6.1

--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] i2c: at91: fix write transfers by clearing pending interrupt first

[Peter Rosin]

On 2015-10-20 15:27, Ludovic Desroches wrote:
> On Mon, Oct 19, 2015 at 12:49:03PM +0200, Peter Rosin wrote:
>> On 2015-10-19 10:51, Ludovic Desroches wrote:
>>> Hi Peter,
>>>
>>> On Fri, Oct 16, 2015 at 11:08:42AM +0200, Peter Rosin wrote:
 On 2015-10-16 01:47, Peter Rosin wrote:
> On 2015-10-14 07:43, Ludovic Desroches wrote:
>> On Tue, Oct 13, 2015 at 08:01:34PM +0200, Peter Rosin wrote:
>>> On 2015-10-13 18:47, Cyrille Pitchen wrote:
 Le 13/10/2015 17:19, Peter Rosin a écrit :
> On 2015-10-13 16:21, Ludovic Desroches wrote:
>>>
>>> [...]
>>>
> I have started to get this when I run with this patch:
>
> [   73.31] at91_i2c f0014000.i2c: RXRDY still set!
> [  198.20] at91_i2c f0014000.i2c: RXRDY still set!
> [  509.88] at91_i2c f0014000.i2c: RXRDY still set!
> [  615.75] at91_i2c f0014000.i2c: RXRDY still set!
> [  617.75] at91_i2c f0014000.i2c: RXRDY still set!
> [ 1766.64] at91_i2c f0014000.i2c: RXRDY still set!
> [ 2035.38] at91_i2c f0014000.i2c: RXRDY still set!
> [ 2227.19] at91_i2c f0014000.i2c: RXRDY still set!
> [ 2313.10] at91_i2c f0014000.i2c: RXRDY still set!
>
> My USB serial dongle was hung which was why I didn't notice until just 
> now.
>
> This is probably not when communication with the eeprom though, and 
> certainly not
> writing to it, but perhpaps when polling the temperature (using the jc42 
> driver).
> I'll investigate further in the morning to see if I can pinpoint it.

 Yep, it's the jc42 accesses that triggers this (to the same chip as the
 eeprom, but a different block of transistors I suppose).

 Looking at the i2c bus, the accesses normally go like this:

 [0.00] S 0x18 W 0x05 S 0x18 R 0xc1 0xbe NACK P
 [0.000521] S 0x18 W 0x04 S 0x18 R 0x00 0x00 NACK P
 [0.001024] S 0x18 W 0x03 S 0x18 R 0x00 0x00 NACK P
 [0.001524] S 0x18 W 0x02 S 0x18 R 0x00 0x00 NACK P
 [0.196991] S 0x18 W 0x05 S 0x18 R 0xc1 0xbe NACK P
 [0.197514] S 0x18 W 0x04 S 0x18 R 0x00 0x00 NACK P
 [0.198019] S 0x18 W 0x03 S 0x18 R 0x00 0x00 NACK P
 [0.198520] S 0x18 W 0x02 S 0x18 R 0x00 0x00 NACK P

 I.e. chunks of four transfers every ~200 ms (I removed the 1Hz rate
 limiter in the jc42 driver to get more frequent incidents).

 But when the diagnostic (RXRDY still set!) is output it continues
 with this:

 [0.399755] S 0x18 W 0x05 S 0x18 R 0xc1 0xbe NACK P
 [0.404998] S 0x18 W 0x04 S 0x18 R 0x00 0x00 NACK P
 [0.405508] S 0x18 W 0x03 S 0x18 R 0x00 0x00 NACK P
 [0.406008] S 0x18 W 0x02 S 0x18 R 0x00 0x00 NACK P

 Notice the ~5 ms delay (about the time it should take to output
 the diagnostic at 115200 baud) after the access to register 0x05
 at 0.399755.

 This is the only thing that I can observe on the bus, so it appears
 to be harmless.

 It appears that the i2c access at 0.399755 finds the TWI
 registers in an odd state, but nothing from the access at
 0.198520 appears to have gone wrong. Is this a race? Anyway,
 the diagnostic is pretty frequent and annoying. printk_once?
>>>
>>> I'll try to reproduce it on my side. The only issue you have is having
>>> the message about RXRDY? I mean no issue with i2c transfers?
>>
>> Exactly, the only issue is the message, the bus looks good and transfers
>> work as they should AFAICT.
>>> It is not the possible bug we had in mind, this bug will prevent the
>>> master device to release the i2c bus. It will stop the transfer but
>>> without sending a stop on the bus.
>>
>> Agreed, this is not about the extra frame caused by the spurious write
>> to the THR register. This is something else.
>>
>> One suspicion is that the driver gets an unexpected irq from its own
>> NACK (the one that it puts out to end the read) and races with the
>> expected interrupt at TXCOMP?
>>
> 
> I have discussed with the designer of this IP. So the NACK flag is only
> used for 'real' nack not protocol ones.
> 
> Concerning the read issue, it have attached a patch (apply it on top of
> Cyrille's patch). Could you have a try? I have reproduced your issue only one
> time so it's hard for me to tell if it works well or not.

Since you removed the dev_err call, I don't really know if the underlying
issue is solved or not (if there ever was any).

Reading RHR in the if at the top of at91_twi_read_next_byte() never happens
for me (I added a witness and left it running overnight), so that
seems like an unrelated change.

Checking RXRDY before TXCOMP|NACK seems about equal, but what do I know?

But the "problem" is gone of course, no flood of messages, but I can't tell
if this is just papering over or a fix.

Cheers,
Peter
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] i2c: designware: register clkdev during acpi device configuration

[Mika Westerberg]

On Wed, Oct 21, 2015 at 05:50:02PM +0800, Ken Xue wrote:
> After apd was accept in kernel V4.1, there is no issue. But between 3.18
> and V4.1, there will be a problem.

We care about that because?
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 1/1] i2c: add ACPI support for I2C mux ports

[Mika Westerberg]

On Wed, Oct 21, 2015 at 01:52:41AM -0700, Dustin Byford wrote:
> On Wed Oct 21 11:34, Mika Westerberg wrote:
> > On Wed, Oct 21, 2015 at 01:21:16AM -0700, Dustin Byford wrote:
> > > On Wed Oct 21 11:12, Mika Westerberg wrote:
> > > > On Tue, Oct 20, 2015 at 10:49:59AM -0700, Dustin Byford wrote:
> > > > > I considered it, but I thought a default that fairly closely matches 
> > > > > the
> > > > > old behavior was more convenient.
> > > > > 
> > > > > On the other hand, leaving it up to the controllers makes it all very
> > > > > explicit and perhaps simpler to reason about.
> > > > > 
> > > > > 
> > > > > I could be convinced either way.  But, if we move it to the controller
> > > > > drivers, which ones need the change?
> > > > > 
> > > > > grep -i acpi drivers/i2c/busses/i2c*
> > > > > 
> > > > > shows 18 drivers that might care.
> > > > 
> > > > I'm quite confident the designware I2C is enough for now. Intel uses it
> > > > for all SoCs with LPSS and I think AMD has the same block for their I2C
> > > > solution.
> > > 
> > > I certainly care about i801, ismt, and isch.  Doesn't this affect any
> > > i2c controller with clients that may be enumerated through ACPI?
> > 
> > Yes, but so far I haven't seen any other devices being used for this
> > than the I2C designware.
> > 
> > Which hardware you are testing this patch on?
> 
> I'm working with a number of x86-based network switch platforms.  Mostly
> rangeley at the moment, but I'm sure others are in the works.  Have a
> look at:
> 
> http://www.opencompute.org/wiki/Networking/SpecsAndDesigns
> 
> for examples.

Cool :)

> My goal, hence the recent patches, is to help the network switch
> industry move a lot of platform description into ACPI.  That means lots
> of complicated I2C trees; switches are full of I2C devices.

I see.

I don't really have strong feelings whether it should be the I2C core or
individual drivers setting the ACPI companion. However, it would be nice
to match DT here and they assign their of_node per driver.
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] i2c: designware: fix platform driver probe rename

[Arnd Bergmann]

A function rename was incomplete and missed the !CONFIG_PM
case:

i2c-designware-platdrv.c:340:13: error: 'dw_i2c_plat_prepare' undeclared here 
(not in a function)
i2c-designware-platdrv.c:341:14: error: 'dw_i2c_plat_complete' undeclared here 
(not in a function)

This renames the macros accordingly.

Signed-off-by: Arnd Bergmann 
Fixes: 6ad6fde3970c ("i2c: designware: Rename platform driver probe and PM 
functions")
---
Found on ARM randconfig builds

diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c 
b/drivers/i2c/busses/i2c-designware-platdrv.c
index f6b252a8ffd1..eb55760ccd9f 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -307,8 +307,8 @@ static void dw_i2c_plat_complete(struct device *dev)
pm_request_resume(dev);
 }
 #else
-#define dw_i2c_prepare NULL
-#define dw_i2c_completeNULL
+#define dw_i2c_plat_prepareNULL
+#define dw_i2c_plat_complete   NULL
 #endif
 
 #ifdef CONFIG_PM

--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] i2c: designware: register clkdev during acpi device configuration

[Mika Westerberg]

On Wed, Oct 21, 2015 at 04:42:23PM +0800, Ken Xue wrote:
> On Wed, 2015-10-21 at 10:28 +0300, Mika Westerberg wrote:
> > On Wed, Oct 21, 2015 at 09:11:33AM +0800, Ken Xue wrote:
> > > On Tue, 2015-10-20 at 14:17 +0300, Mika Westerberg wrote:
> > > > On Tue, Oct 20, 2015 at 02:38:01PM +0800, Ken Xue wrote:
> > > > > DW I2C driver tries to register a clk from id->driver_data as an
> > > > > alternative way besides intel lpss. But code doesn't register the
> > > > > clk to clkdev. So, devm_clk_get will fail during probe.
> > > > > 
> > > > > The patch can fix this issue.
> > > > 
> > > > Since you now have drivers/acpi/acpi_apd.c for AMD ACPI stuff, can you
> > > > create the clock there just like we do for Intel stuff?
> > > Sure. APD already creates the clock for AMD0010 as you expected. And the
> > > next patch([PATCH 2/2] i2c: designware: remove freq definition for
> > > "AMD0010" in acpi_device_id) is dropping the old way for getting freq.
> > 
> > So this patch is not necessary, right?
> Even though there is no use case that getting freq from id->driver_data,
> But if we want to keep this design, then we should use current patch for
> fixing the potential issue. So, the patch is nice to have.

What potential issue?

If you pass clock from drivers/acpi/acpi_apd.c and drop the hard coded
freq for AMD0010 in the I2C designware driver, the driver still works
just fine.

> Otherwise, we have to revert whole old design(a445900c).

Yes please :-)
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] i2c: designware: register clkdev during acpi device configuration

[Ken Xue]

On Wed, 2015-10-21 at 12:25 +0300, Mika Westerberg wrote:
> On Wed, Oct 21, 2015 at 04:42:23PM +0800, Ken Xue wrote:
> > On Wed, 2015-10-21 at 10:28 +0300, Mika Westerberg wrote:
> > > On Wed, Oct 21, 2015 at 09:11:33AM +0800, Ken Xue wrote:
> > > > On Tue, 2015-10-20 at 14:17 +0300, Mika Westerberg wrote:
> > > > > On Tue, Oct 20, 2015 at 02:38:01PM +0800, Ken Xue wrote:
> > > > > > DW I2C driver tries to register a clk from id->driver_data as an
> > > > > > alternative way besides intel lpss. But code doesn't register the
> > > > > > clk to clkdev. So, devm_clk_get will fail during probe.
> > > > > > 
> > > > > > The patch can fix this issue.
> > > > > 
> > > > > Since you now have drivers/acpi/acpi_apd.c for AMD ACPI stuff, can you
> > > > > create the clock there just like we do for Intel stuff?
> > > > Sure. APD already creates the clock for AMD0010 as you expected. And the
> > > > next patch([PATCH 2/2] i2c: designware: remove freq definition for
> > > > "AMD0010" in acpi_device_id) is dropping the old way for getting freq.
> > > 
> > > So this patch is not necessary, right?
> > Even though there is no use case that getting freq from id->driver_data,
> > But if we want to keep this design, then we should use current patch for
> > fixing the potential issue. So, the patch is nice to have.
> 
> What potential issue?
devm_clk_get will fail during probe for AMD0010 without current patch.

> 
> If you pass clock from drivers/acpi/acpi_apd.c and drop the hard coded
> freq for AMD0010 in the I2C designware driver, the driver still works
> just fine.
> 
> > Otherwise, we have to revert whole old design(a445900c).
> 
> Yes please :-)
Glad to do.


--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] i2c: designware: register clkdev during acpi device configuration

[Mika Westerberg]

On Wed, Oct 21, 2015 at 05:37:53PM +0800, Ken Xue wrote:
> On Wed, 2015-10-21 at 12:25 +0300, Mika Westerberg wrote:
> > On Wed, Oct 21, 2015 at 04:42:23PM +0800, Ken Xue wrote:
> > > On Wed, 2015-10-21 at 10:28 +0300, Mika Westerberg wrote:
> > > > On Wed, Oct 21, 2015 at 09:11:33AM +0800, Ken Xue wrote:
> > > > > On Tue, 2015-10-20 at 14:17 +0300, Mika Westerberg wrote:
> > > > > > On Tue, Oct 20, 2015 at 02:38:01PM +0800, Ken Xue wrote:
> > > > > > > DW I2C driver tries to register a clk from id->driver_data as an
> > > > > > > alternative way besides intel lpss. But code doesn't register the
> > > > > > > clk to clkdev. So, devm_clk_get will fail during probe.
> > > > > > > 
> > > > > > > The patch can fix this issue.
> > > > > > 
> > > > > > Since you now have drivers/acpi/acpi_apd.c for AMD ACPI stuff, can 
> > > > > > you
> > > > > > create the clock there just like we do for Intel stuff?
> > > > > Sure. APD already creates the clock for AMD0010 as you expected. And 
> > > > > the
> > > > > next patch([PATCH 2/2] i2c: designware: remove freq definition for
> > > > > "AMD0010" in acpi_device_id) is dropping the old way for getting freq.
> > > > 
> > > > So this patch is not necessary, right?
> > > Even though there is no use case that getting freq from id->driver_data,
> > > But if we want to keep this design, then we should use current patch for
> > > fixing the potential issue. So, the patch is nice to have.
> > 
> > What potential issue?
> devm_clk_get will fail during probe for AMD0010 without current patch.

How can it fail if you provide the very clock from drivers/acpi/acpi_apd.c?

> > 
> > If you pass clock from drivers/acpi/acpi_apd.c and drop the hard coded
> > freq for AMD0010 in the I2C designware driver, the driver still works
> > just fine.
> > 
> > > Otherwise, we have to revert whole old design(a445900c).
> > 
> > Yes please :-)
> Glad to do.

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] i2c: designware: register clkdev during acpi device configuration

[Ken Xue]

On Wed, 2015-10-21 at 12:49 +0300, Mika Westerberg wrote:
> On Wed, Oct 21, 2015 at 05:37:53PM +0800, Ken Xue wrote:
> > On Wed, 2015-10-21 at 12:25 +0300, Mika Westerberg wrote:
> > > On Wed, Oct 21, 2015 at 04:42:23PM +0800, Ken Xue wrote:
> > > > On Wed, 2015-10-21 at 10:28 +0300, Mika Westerberg wrote:
> > > > > On Wed, Oct 21, 2015 at 09:11:33AM +0800, Ken Xue wrote:
> > > > > > On Tue, 2015-10-20 at 14:17 +0300, Mika Westerberg wrote:
> > > > > > > On Tue, Oct 20, 2015 at 02:38:01PM +0800, Ken Xue wrote:
> > > > > > > > DW I2C driver tries to register a clk from id->driver_data as an
> > > > > > > > alternative way besides intel lpss. But code doesn't register 
> > > > > > > > the
> > > > > > > > clk to clkdev. So, devm_clk_get will fail during probe.
> > > > > > > > 
> > > > > > > > The patch can fix this issue.
> > > > > > > 
> > > > > > > Since you now have drivers/acpi/acpi_apd.c for AMD ACPI stuff, 
> > > > > > > can you
> > > > > > > create the clock there just like we do for Intel stuff?
> > > > > > Sure. APD already creates the clock for AMD0010 as you expected. 
> > > > > > And the
> > > > > > next patch([PATCH 2/2] i2c: designware: remove freq definition for
> > > > > > "AMD0010" in acpi_device_id) is dropping the old way for getting 
> > > > > > freq.
> > > > > 
> > > > > So this patch is not necessary, right?
> > > > Even though there is no use case that getting freq from id->driver_data,
> > > > But if we want to keep this design, then we should use current patch for
> > > > fixing the potential issue. So, the patch is nice to have.
> > > 
> > > What potential issue?
> > devm_clk_get will fail during probe for AMD0010 without current patch.
> 
> How can it fail if you provide the very clock from drivers/acpi/acpi_apd.c?
After apd was accept in kernel V4.1, there is no issue. But between 3.18
and V4.1, there will be a problem.

--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] dts: keystone: Use new "ti,keystone-i2c" compatible

[Sekhar Nori]

On Tuesday 20 October 2015 09:40 PM, Grygorii Strashko wrote:
> On 10/20/2015 06:06 PM, Wolfram Sang wrote:
>> On Mon, Sep 14, 2015 at 11:07:02AM +0200, Alexander Sverdlin wrote:
>>> Now as "i2c-davinci" driver has special handling for Keystone it's
>>> time to switch
>>> the device tree to use new "compatible" property. Old one is left for
>>> backwards-
>>> compatibility.
>>>
>>> Signed-off-by: Alexander Sverdlin 
>>
>> Shall I take this one or shall it go via the davinci tree?

I don't have the patch in my mailbox now. But as I remember this is a
driver only patch and should go via i2c tree.

Thanks,
Sekhar

--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] i2c: designware: register clkdev during acpi device configuration

[Ken Xue]

On Wed, 2015-10-21 at 10:28 +0300, Mika Westerberg wrote:
> On Wed, Oct 21, 2015 at 09:11:33AM +0800, Ken Xue wrote:
> > On Tue, 2015-10-20 at 14:17 +0300, Mika Westerberg wrote:
> > > On Tue, Oct 20, 2015 at 02:38:01PM +0800, Ken Xue wrote:
> > > > DW I2C driver tries to register a clk from id->driver_data as an
> > > > alternative way besides intel lpss. But code doesn't register the
> > > > clk to clkdev. So, devm_clk_get will fail during probe.
> > > > 
> > > > The patch can fix this issue.
> > > 
> > > Since you now have drivers/acpi/acpi_apd.c for AMD ACPI stuff, can you
> > > create the clock there just like we do for Intel stuff?
> > Sure. APD already creates the clock for AMD0010 as you expected. And the
> > next patch([PATCH 2/2] i2c: designware: remove freq definition for
> > "AMD0010" in acpi_device_id) is dropping the old way for getting freq.
> 
> So this patch is not necessary, right?
Even though there is no use case that getting freq from id->driver_data,
But if we want to keep this design, then we should use current patch for
fixing the potential issue. So, the patch is nice to have.

Otherwise, we have to revert whole old design(a445900c).

--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] i2c: designware: register clkdev during acpi device configuration

[Mika Westerberg]

On Wed, Oct 21, 2015 at 05:50:02PM +0800, Ken Xue wrote:
> On Wed, 2015-10-21 at 12:49 +0300, Mika Westerberg wrote:
> > On Wed, Oct 21, 2015 at 05:37:53PM +0800, Ken Xue wrote:
> > > On Wed, 2015-10-21 at 12:25 +0300, Mika Westerberg wrote:
> > > > On Wed, Oct 21, 2015 at 04:42:23PM +0800, Ken Xue wrote:
> > > > > On Wed, 2015-10-21 at 10:28 +0300, Mika Westerberg wrote:
> > > > > > On Wed, Oct 21, 2015 at 09:11:33AM +0800, Ken Xue wrote:
> > > > > > > On Tue, 2015-10-20 at 14:17 +0300, Mika Westerberg wrote:
> > > > > > > > On Tue, Oct 20, 2015 at 02:38:01PM +0800, Ken Xue wrote:
> > > > > > > > > DW I2C driver tries to register a clk from id->driver_data as 
> > > > > > > > > an
> > > > > > > > > alternative way besides intel lpss. But code doesn't register 
> > > > > > > > > the
> > > > > > > > > clk to clkdev. So, devm_clk_get will fail during probe.
> > > > > > > > > 
> > > > > > > > > The patch can fix this issue.
> > > > > > > > 
> > > > > > > > Since you now have drivers/acpi/acpi_apd.c for AMD ACPI stuff, 
> > > > > > > > can you
> > > > > > > > create the clock there just like we do for Intel stuff?
> > > > > > > Sure. APD already creates the clock for AMD0010 as you expected. 
> > > > > > > And the
> > > > > > > next patch([PATCH 2/2] i2c: designware: remove freq definition for
> > > > > > > "AMD0010" in acpi_device_id) is dropping the old way for getting 
> > > > > > > freq.
> > > > > > 
> > > > > > So this patch is not necessary, right?
> > > > > Even though there is no use case that getting freq from 
> > > > > id->driver_data,
> > > > > But if we want to keep this design, then we should use current patch 
> > > > > for
> > > > > fixing the potential issue. So, the patch is nice to have.
> > > > 
> > > > What potential issue?
> > > devm_clk_get will fail during probe for AMD0010 without current patch.
> > 
> > How can it fail if you provide the very clock from drivers/acpi/acpi_apd.c?
> After apd was accept in kernel V4.1, there is no issue. But between 3.18
> and V4.1, there will be a problem.

Ah, now I got it.

You are saying that the original commit a445900c906092 ("i2c:
designware: Add support for AMD I2C controller") actually never worked
because it failed to register the clock with clkdev? In that case it is
not even a regression ;-) Oh my...

In that case I don't think we need to fix that for 3.18+ because it
never worked in the first place.
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 1/1] i2c: add ACPI support for I2C mux ports

[Dustin Byford]

On Wed Oct 21 12:08, Mika Westerberg wrote:
> I don't really have strong feelings whether it should be the I2C core or
> individual drivers setting the ACPI companion. However, it would be nice
> to match DT here and they assign their of_node per driver.

OK with me, if we can convince Rafael this is a good idea, I'll send a
new revision with drivers setting the companion.

--Dustin
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] i2c: designware: fix platform driver probe rename

[Andy Shevchenko]

On Wed, 2015-10-21 at 11:16 +0200, Arnd Bergmann wrote:
> A function rename was incomplete and missed the !CONFIG_PM
> case:
> 
> i2c-designware-platdrv.c:340:13: error: 'dw_i2c_plat_prepare'
> undeclared here (not in a function)
> i2c-designware-platdrv.c:341:14: error: 'dw_i2c_plat_complete'
> undeclared here (not in a function)
> 
> This renames the macros accordingly.

Jarkko sent this already couple hours ago.

> 
> Signed-off-by: Arnd Bergmann 
> Fixes: 6ad6fde3970c ("i2c: designware: Rename platform driver probe
> and PM functions")
> ---
> Found on ARM randconfig builds
> 
> diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c
> b/drivers/i2c/busses/i2c-designware-platdrv.c
> index f6b252a8ffd1..eb55760ccd9f 100644
> --- a/drivers/i2c/busses/i2c-designware-platdrv.c
> +++ b/drivers/i2c/busses/i2c-designware-platdrv.c
> @@ -307,8 +307,8 @@ static void dw_i2c_plat_complete(struct device
> *dev)
>   pm_request_resume(dev);
>  }
>  #else
> -#define dw_i2c_prepare   NULL
> -#define dw_i2c_complete  NULL
> +#define dw_i2c_plat_prepare  NULL
> +#define dw_i2c_plat_complete NULL
>  #endif
>  
>  #ifdef CONFIG_PM
> 

-- 
Andy Shevchenko 
Intel Finland Oy

--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] dts: keystone: Use new "ti,keystone-i2c" compatible

[Grygorii Strashko]

On 09/14/2015 12:07 PM, Alexander Sverdlin wrote:

Now as "i2c-davinci" driver has special handling for Keystone it's time to 
switch
the device tree to use new "compatible" property. Old one is left for backwards-
compatibility.

Signed-off-by: Alexander Sverdlin 
---


To: Santosh
Cc: Murali, Sekhar

Seems this patch is Keystone 2 specific.



Changes in v2:
- As suggested by Mark Rutland, kept the old "compatible" for backwards-
   compatibility

  arch/arm/boot/dts/keystone.dtsi |6 +++---
  1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm/boot/dts/keystone.dtsi b/arch/arm/boot/dts/keystone.dtsi
index 72816d6..abd7455 100644
--- a/arch/arm/boot/dts/keystone.dtsi
+++ b/arch/arm/boot/dts/keystone.dtsi
@@ -106,7 +106,7 @@
};

i2c0: i2c@253 {
-   compatible = "ti,davinci-i2c";
+   compatible = "ti,keystone-i2c", "ti,davinci-i2c";
reg = <0x0253 0x400>;
clock-frequency = <10>;
clocks = <>;
@@ -116,7 +116,7 @@
};

i2c1: i2c@2530400 {
-   compatible = "ti,davinci-i2c";
+   compatible = "ti,keystone-i2c", "ti,davinci-i2c";
reg = <0x02530400 0x400>;
clock-frequency = <10>;
clocks = <>;
@@ -126,7 +126,7 @@
};

i2c2: i2c@2530800 {
-   compatible = "ti,davinci-i2c";
+   compatible = "ti,keystone-i2c", "ti,davinci-i2c";
reg = <0x02530800 0x400>;
clock-frequency = <10>;
clocks = <>;




--
regards,
-grygorii
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RESEND PATCH 0/9] eeprom: at24: at24cs series serial number read

[Peter Korsgaard]

> "Bartosz" == Bartosz Golaszewski  writes:

 > Chips from the at24cs EEPROM series have an additional read-only memory area
 > containing a factory pre-programmed serial number. In order to access it, a
 > dummy write must be executed before reading the serial number bytes.

 > This series adds support for reading the serial number through a sysfs
 > attribute.

 > While we're at it: some of the patches contain readability tweaks and code
 > organization fixes.

 > Tested with at24cs64 and at24cs02 chips (for both 16 and 8 bit address
 > pointers).

As the serial number is available on a separate i2c address, wouldn't
it be simpler to handle these as special (read only) device variants and
instantiate E.G. a 24c64 (for the normal data) and a 24cs64 (for the
serial)?

-- 
Bye, Peter Korsgaard
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] i2c: at91: fix write transfers by clearing pending interrupt first

[Ludovic Desroches]

On Wed, Oct 21, 2015 at 09:42:40AM +0200, Peter Rosin wrote:
> On 2015-10-21 09:21, Peter Rosin wrote:
> > On 2015-10-20 15:27, Ludovic Desroches wrote:
> >> On Mon, Oct 19, 2015 at 12:49:03PM +0200, Peter Rosin wrote:
> >>> On 2015-10-19 10:51, Ludovic Desroches wrote:
>  Hi Peter,
> 
>  On Fri, Oct 16, 2015 at 11:08:42AM +0200, Peter Rosin wrote:
> > On 2015-10-16 01:47, Peter Rosin wrote:
> >> On 2015-10-14 07:43, Ludovic Desroches wrote:
> >>> On Tue, Oct 13, 2015 at 08:01:34PM +0200, Peter Rosin wrote:
>  On 2015-10-13 18:47, Cyrille Pitchen wrote:
> > Le 13/10/2015 17:19, Peter Rosin a écrit :
> >> On 2015-10-13 16:21, Ludovic Desroches wrote:
> 
>  [...]
> 
> >> I have started to get this when I run with this patch:
> >>
> >> [   73.31] at91_i2c f0014000.i2c: RXRDY still set!
> >> [  198.20] at91_i2c f0014000.i2c: RXRDY still set!
> >> [  509.88] at91_i2c f0014000.i2c: RXRDY still set!
> >> [  615.75] at91_i2c f0014000.i2c: RXRDY still set!
> >> [  617.75] at91_i2c f0014000.i2c: RXRDY still set!
> >> [ 1766.64] at91_i2c f0014000.i2c: RXRDY still set!
> >> [ 2035.38] at91_i2c f0014000.i2c: RXRDY still set!
> >> [ 2227.19] at91_i2c f0014000.i2c: RXRDY still set!
> >> [ 2313.10] at91_i2c f0014000.i2c: RXRDY still set!
> >>
> >> My USB serial dongle was hung which was why I didn't notice until just 
> >> now.
> >>
> >> This is probably not when communication with the eeprom though, and 
> >> certainly not
> >> writing to it, but perhpaps when polling the temperature (using the 
> >> jc42 driver).
> >> I'll investigate further in the morning to see if I can pinpoint it.
> >
> > Yep, it's the jc42 accesses that triggers this (to the same chip as the
> > eeprom, but a different block of transistors I suppose).
> >
> > Looking at the i2c bus, the accesses normally go like this:
> >
> > [0.00] S 0x18 W 0x05 S 0x18 R 0xc1 0xbe NACK P
> > [0.000521] S 0x18 W 0x04 S 0x18 R 0x00 0x00 NACK P
> > [0.001024] S 0x18 W 0x03 S 0x18 R 0x00 0x00 NACK P
> > [0.001524] S 0x18 W 0x02 S 0x18 R 0x00 0x00 NACK P
> > [0.196991] S 0x18 W 0x05 S 0x18 R 0xc1 0xbe NACK P
> > [0.197514] S 0x18 W 0x04 S 0x18 R 0x00 0x00 NACK P
> > [0.198019] S 0x18 W 0x03 S 0x18 R 0x00 0x00 NACK P
> > [0.198520] S 0x18 W 0x02 S 0x18 R 0x00 0x00 NACK P
> >
> > I.e. chunks of four transfers every ~200 ms (I removed the 1Hz rate
> > limiter in the jc42 driver to get more frequent incidents).
> >
> > But when the diagnostic (RXRDY still set!) is output it continues
> > with this:
> >
> > [0.399755] S 0x18 W 0x05 S 0x18 R 0xc1 0xbe NACK P
> > [0.404998] S 0x18 W 0x04 S 0x18 R 0x00 0x00 NACK P
> > [0.405508] S 0x18 W 0x03 S 0x18 R 0x00 0x00 NACK P
> > [0.406008] S 0x18 W 0x02 S 0x18 R 0x00 0x00 NACK P
> >
> > Notice the ~5 ms delay (about the time it should take to output
> > the diagnostic at 115200 baud) after the access to register 0x05
> > at 0.399755.
> >
> > This is the only thing that I can observe on the bus, so it appears
> > to be harmless.
> >
> > It appears that the i2c access at 0.399755 finds the TWI
> > registers in an odd state, but nothing from the access at
> > 0.198520 appears to have gone wrong. Is this a race? Anyway,
> > the diagnostic is pretty frequent and annoying. printk_once?
> 
>  I'll try to reproduce it on my side. The only issue you have is having
>  the message about RXRDY? I mean no issue with i2c transfers?
> >>>
> >>> Exactly, the only issue is the message, the bus looks good and transfers
> >>> work as they should AFAICT.
>  It is not the possible bug we had in mind, this bug will prevent the
>  master device to release the i2c bus. It will stop the transfer but
>  without sending a stop on the bus.
> >>>
> >>> Agreed, this is not about the extra frame caused by the spurious write
> >>> to the THR register. This is something else.
> >>>
> >>> One suspicion is that the driver gets an unexpected irq from its own
> >>> NACK (the one that it puts out to end the read) and races with the
> >>> expected interrupt at TXCOMP?
> >>>
> >>
> >> I have discussed with the designer of this IP. So the NACK flag is only
> >> used for 'real' nack not protocol ones.
> >>
> >> Concerning the read issue, it have attached a patch (apply it on top of
> >> Cyrille's patch). Could you have a try? I have reproduced your issue only 
> >> one
> >> time so it's hard for me to tell if it works well or not.
> > 
> > Since you removed the dev_err call, I don't really know if the underlying
> > issue is solved or not (if there ever was any).
> > 
> > Reading RHR in the if at the top of at91_twi_read_next_byte() never happens
> > for me (I added a witness and left it running overnight), so that
> > seems 

Re: [PATCH v2] dts: keystone: Use new "ti,keystone-i2c" compatible

[Alexander Sverdlin]

Hi!

On 21/10/15 12:13, EXT Sekhar Nori wrote:
 Now as "i2c-davinci" driver has special handling for Keystone it's
 >>> time to switch
 >>> the device tree to use new "compatible" property. Old one is left for
 >>> backwards-
 >>> compatibility.
 >>>
 >>> Signed-off-by: Alexander Sverdlin 
>>> >>
>>> >> Shall I take this one or shall it go via the davinci tree?
> I don't have the patch in my mailbox now. But as I remember this is a

You can review it here:
http://patchwork.ozlabs.org/patch/517321/

> driver only patch and should go via i2c tree.

It's a DT change, because we agreed (with Grygorii) that new "compatible" is 
more
future proof.

-- 
Best regards,
Alexander Sverdlin.
Sent from my pdp-11
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [GIT PULL] On-demand device probing

[Jean-Francois Moine]

Sorry to enter this thread a bit late.

About the number of probe deferred messages, I proposed a simple patch to
reduce them:

https://lkml.org/lkml/2013/8/20/218

I was wondering how many messages this patch could save...

-- 
Ken ar c'hentañ | ** Breizh ha Linux atav! **
Jef |   http://moinejf.free.fr/
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] i2c: mv64xxx: really allow I2C offloading

[Wolfram Sang]

> Wolfram, do you want me to send a v2 with Hezi's full name or will you
> fix that up when applying the patch ?

I'll fix.



signature.asc
Description: Digital signature

Re: [PATCH v3 1/1] i2c: add ACPI support for I2C mux ports

[Mika Westerberg]

On Wed, Oct 21, 2015 at 01:12:01AM +0200, Rafael J. Wysocki wrote:
> Well, we already have that in the MFD case, but in principle it may be
> problematic for things like power management (say you want to put a
> child device into D3, so you use _PS3 on its ACPI companion and then
> the parent is powere down instead).

That case I understand and we should not allow that. However, here we
have an I2C adapter which is purely Linux abstraction that does not have
any representation either in DT nor ACPI. And we don't do any power
management for that either.

If I understand correctly, DT shares the same of_node for both the host
controller device and the adapter which then makes it possible to
enumerate devices behind by just looking adap->dev.of_node. In case of
ACPI we need to know that it's the parent device that we should look for
child devices which may not be true always (like what this patch is
trying to resolve).

> At least, devices in that setup should not be attached to the ACPI PM
> domain.

Agreed.

Currently acpi_dev_pm_attach() only attaches the first physical device
to the ACPI power domain so this should be taken care already.
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 1/1] i2c: add ACPI support for I2C mux ports

[Mika Westerberg]

On Tue, Oct 20, 2015 at 10:49:59AM -0700, Dustin Byford wrote:
> I considered it, but I thought a default that fairly closely matches the
> old behavior was more convenient.
> 
> On the other hand, leaving it up to the controllers makes it all very
> explicit and perhaps simpler to reason about.
> 
> 
> I could be convinced either way.  But, if we move it to the controller
> drivers, which ones need the change?
> 
> grep -i acpi drivers/i2c/busses/i2c*
> 
> shows 18 drivers that might care.

I'm quite confident the designware I2C is enough for now. Intel uses it
for all SoCs with LPSS and I think AMD has the same block for their I2C
solution.

> > adap->dev.parent = >dev;
> > adap->dev.of_node = pdev->dev.of_node;
> > ACPI_COMPANION_SET(>dev, ACPI_COMPANION(>dev));
> 
> Interesting, this code isn't in my tree.  I wonder why it was added,
> what code looks at the acpi companion on the i2c dev?  Before my change
> it was supposed to be NULL, and it is NULL on every other controller.

It is not in any tree. I meant that before b34bb1ee71158d5b it looked
something like that :-)
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Alternative approach to solve the deferred probe

[Russell King - ARM Linux]

On Tue, Oct 20, 2015 at 08:58:19PM -0700, Frank Rowand wrote:
> On 10/20/2015 8:46 AM, Russell King - ARM Linux wrote:
> > On Mon, Oct 19, 2015 at 06:21:40PM +0200, Geert Uytterhoeven wrote:
> >> Hi Russell,
> >>
> >> On Mon, Oct 19, 2015 at 5:35 PM, Russell King - ARM Linux
> >>  wrote:
> > What you can do is print those devices which have failed to probe at
> > late_initcall() time - possibly augmenting that with reports from
> > subsystems showing what resources are not available, but that's only
> > a guide, because of the "it might or might not be in a kernel module"
> > problem.
> 
>  Well, adding those reports would give you a changelog similar to the
>  one in this series...
> >>>
> >>> I'm not sure about that, because what I was thinking of is adding
> >>> a flag which would be set at late_initcall() time prior to running
> >>> a final round of deferred device probing.
> >>
> >> Which round is the final round?
> >> That's the one which didn't manage to bind any new devices to drivers,
> >> which is something you only know _after_ the round has been run.
> >>
> >> So I think we need one extra round to handle this.
> >>
> >>> This flag would then be used in a deferred_warn() printk function
> >>> which would normally be silent, but when this flag is set, it would
> >>> print the reason for the deferral - and this would replace (or be
> >>> added) to the subsystems and drivers which return -EPROBE_DEFER.
> >>>
> >>> That has the effect of hiding all the deferrals up until just before
> >>> launching into userspace, which should then acomplish two things -
> >>> firstly, getting rid of the rather useless deferred messages up to
> >>> that point, and secondly printing the reason why the remaining
> >>> deferrals are happening.
> >>>
> >>> That should be a small number of new lines plus a one-line change
> >>> in subsystems and drivers.
> >>
> >> Apart from the extra round we probably can't get rid of, that sounds OK to 
> >> me.
> > 
> > Something like this.  I haven't put a lot of effort into it to change all
> > the places which return an -EPROBE_DEFER, and it also looks like we need
> > some helpers to report when we have only an device_node (or should that
> > be fwnode?)  See the commented out of_warn_deferred() in
> > drivers/gpio/gpiolib-of.c.  Adding this stuff in the subsystems searching
> > for resources should make debugging why things are getting deferred easier.
> > 
> > We could make driver_deferred_probe_report something that can be
> > deactivated again after the last deferred probe run, and provide the
> > user with a knob that they can turn it back on again.
> > 
> > I've tried this out on two of my platforms, including forcing
> > driver_deferred_probe_report to be enabled, and I get exactly one
> > deferred probe, so not a particularly good test.
> > 
> > The patch won't apply as-is to mainline for all files; it's based on my
> > tree which has some 360 additional patches (which seems to be about
> > normal for my tree now.)
> 
> I like the concept (I have been thinking along similar lines lately).
> But I think this might make the console messages more confusing than
> they are now.

If messages end up being given from the subsystem rather than the driver,
surely they become more consistent?

> The problem is that debug, warn, and error messages
> come from a somewhat random set of locations at the moment.  Some
> come from the driver probe routines and some come from the subsystems
> that the probe routines call.  So the patch is suppressing some
> messages, but not others.

The patch is not complete (read the description above).

> > +void dev_warn_deferred(struct device *dev, const char *fmt, ...)
> > +{
> > +   if (driver_deferred_probe_report) {
> > +   struct va_format vaf;
> > +   va_list ap;
> > +
> > +   va_start(ap, fmt);
> > +   vaf.fmt = fmt;
> > +   vaf.va = 
> > +
> > +   dev_warn(dev, "deferring probe: %pV", );
> > +   va_end(ap);
> > +   }
> > +}
> > +EXPORT_SYMBOL_GPL(dev_warn_deferred);
> 
> The places where dev_warn_deferred() replaces dev_dbg(), we lose the
> ability to turn on debugging and observe the driver reporting the
> specific reason the deferral is occurring. So it would be useful to
> add an "else dev_dbg()" in dev_warn_deferred() to retain that capability.

That's a possibility.

> 
> > +
> >  static bool driver_deferred_probe_enable = false;
> > +
> >  /**
> >   * driver_deferred_probe_trigger() - Kick off re-probing deferred devices
> >   *
> > @@ -188,6 +210,13 @@ static int deferred_probe_initcall(void)
> > driver_deferred_probe_trigger();
> 
> Couldn't you put the "driver_deferred_probe_report = true" here?  And then
> not add another round of probes.

The idea is not to report anything for drivers that were deferred
during the normal bootup.  The above is part of the normal bootup,
and the deferred activity should not be warned 

Re: [PATCH v3 1/1] i2c: add ACPI support for I2C mux ports

[Mika Westerberg]

On Wed, Oct 21, 2015 at 01:21:16AM -0700, Dustin Byford wrote:
> On Wed Oct 21 11:12, Mika Westerberg wrote:
> > On Tue, Oct 20, 2015 at 10:49:59AM -0700, Dustin Byford wrote:
> > > I considered it, but I thought a default that fairly closely matches the
> > > old behavior was more convenient.
> > > 
> > > On the other hand, leaving it up to the controllers makes it all very
> > > explicit and perhaps simpler to reason about.
> > > 
> > > 
> > > I could be convinced either way.  But, if we move it to the controller
> > > drivers, which ones need the change?
> > > 
> > > grep -i acpi drivers/i2c/busses/i2c*
> > > 
> > > shows 18 drivers that might care.
> > 
> > I'm quite confident the designware I2C is enough for now. Intel uses it
> > for all SoCs with LPSS and I think AMD has the same block for their I2C
> > solution.
> 
> I certainly care about i801, ismt, and isch.  Doesn't this affect any
> i2c controller with clients that may be enumerated through ACPI?

Yes, but so far I haven't seen any other devices being used for this
than the I2C designware.

Which hardware you are testing this patch on?
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] i2c: designware: register clkdev during acpi device configuration

[Mika Westerberg]

On Wed, Oct 21, 2015 at 09:11:33AM +0800, Ken Xue wrote:
> On Tue, 2015-10-20 at 14:17 +0300, Mika Westerberg wrote:
> > On Tue, Oct 20, 2015 at 02:38:01PM +0800, Ken Xue wrote:
> > > DW I2C driver tries to register a clk from id->driver_data as an
> > > alternative way besides intel lpss. But code doesn't register the
> > > clk to clkdev. So, devm_clk_get will fail during probe.
> > > 
> > > The patch can fix this issue.
> > 
> > Since you now have drivers/acpi/acpi_apd.c for AMD ACPI stuff, can you
> > create the clock there just like we do for Intel stuff?
> Sure. APD already creates the clock for AMD0010 as you expected. And the
> next patch([PATCH 2/2] i2c: designware: remove freq definition for
> "AMD0010" in acpi_device_id) is dropping the old way for getting freq.

So this patch is not necessary, right?
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] i2c: at91: fix write transfers by clearing pending interrupt first

[Peter Rosin]

On 2015-10-21 09:21, Peter Rosin wrote:
> On 2015-10-20 15:27, Ludovic Desroches wrote:
>> On Mon, Oct 19, 2015 at 12:49:03PM +0200, Peter Rosin wrote:
>>> On 2015-10-19 10:51, Ludovic Desroches wrote:
 Hi Peter,

 On Fri, Oct 16, 2015 at 11:08:42AM +0200, Peter Rosin wrote:
> On 2015-10-16 01:47, Peter Rosin wrote:
>> On 2015-10-14 07:43, Ludovic Desroches wrote:
>>> On Tue, Oct 13, 2015 at 08:01:34PM +0200, Peter Rosin wrote:
 On 2015-10-13 18:47, Cyrille Pitchen wrote:
> Le 13/10/2015 17:19, Peter Rosin a écrit :
>> On 2015-10-13 16:21, Ludovic Desroches wrote:

 [...]

>> I have started to get this when I run with this patch:
>>
>> [   73.31] at91_i2c f0014000.i2c: RXRDY still set!
>> [  198.20] at91_i2c f0014000.i2c: RXRDY still set!
>> [  509.88] at91_i2c f0014000.i2c: RXRDY still set!
>> [  615.75] at91_i2c f0014000.i2c: RXRDY still set!
>> [  617.75] at91_i2c f0014000.i2c: RXRDY still set!
>> [ 1766.64] at91_i2c f0014000.i2c: RXRDY still set!
>> [ 2035.38] at91_i2c f0014000.i2c: RXRDY still set!
>> [ 2227.19] at91_i2c f0014000.i2c: RXRDY still set!
>> [ 2313.10] at91_i2c f0014000.i2c: RXRDY still set!
>>
>> My USB serial dongle was hung which was why I didn't notice until just 
>> now.
>>
>> This is probably not when communication with the eeprom though, and 
>> certainly not
>> writing to it, but perhpaps when polling the temperature (using the jc42 
>> driver).
>> I'll investigate further in the morning to see if I can pinpoint it.
>
> Yep, it's the jc42 accesses that triggers this (to the same chip as the
> eeprom, but a different block of transistors I suppose).
>
> Looking at the i2c bus, the accesses normally go like this:
>
> [0.00] S 0x18 W 0x05 S 0x18 R 0xc1 0xbe NACK P
> [0.000521] S 0x18 W 0x04 S 0x18 R 0x00 0x00 NACK P
> [0.001024] S 0x18 W 0x03 S 0x18 R 0x00 0x00 NACK P
> [0.001524] S 0x18 W 0x02 S 0x18 R 0x00 0x00 NACK P
> [0.196991] S 0x18 W 0x05 S 0x18 R 0xc1 0xbe NACK P
> [0.197514] S 0x18 W 0x04 S 0x18 R 0x00 0x00 NACK P
> [0.198019] S 0x18 W 0x03 S 0x18 R 0x00 0x00 NACK P
> [0.198520] S 0x18 W 0x02 S 0x18 R 0x00 0x00 NACK P
>
> I.e. chunks of four transfers every ~200 ms (I removed the 1Hz rate
> limiter in the jc42 driver to get more frequent incidents).
>
> But when the diagnostic (RXRDY still set!) is output it continues
> with this:
>
> [0.399755] S 0x18 W 0x05 S 0x18 R 0xc1 0xbe NACK P
> [0.404998] S 0x18 W 0x04 S 0x18 R 0x00 0x00 NACK P
> [0.405508] S 0x18 W 0x03 S 0x18 R 0x00 0x00 NACK P
> [0.406008] S 0x18 W 0x02 S 0x18 R 0x00 0x00 NACK P
>
> Notice the ~5 ms delay (about the time it should take to output
> the diagnostic at 115200 baud) after the access to register 0x05
> at 0.399755.
>
> This is the only thing that I can observe on the bus, so it appears
> to be harmless.
>
> It appears that the i2c access at 0.399755 finds the TWI
> registers in an odd state, but nothing from the access at
> 0.198520 appears to have gone wrong. Is this a race? Anyway,
> the diagnostic is pretty frequent and annoying. printk_once?

 I'll try to reproduce it on my side. The only issue you have is having
 the message about RXRDY? I mean no issue with i2c transfers?
>>>
>>> Exactly, the only issue is the message, the bus looks good and transfers
>>> work as they should AFAICT.
 It is not the possible bug we had in mind, this bug will prevent the
 master device to release the i2c bus. It will stop the transfer but
 without sending a stop on the bus.
>>>
>>> Agreed, this is not about the extra frame caused by the spurious write
>>> to the THR register. This is something else.
>>>
>>> One suspicion is that the driver gets an unexpected irq from its own
>>> NACK (the one that it puts out to end the read) and races with the
>>> expected interrupt at TXCOMP?
>>>
>>
>> I have discussed with the designer of this IP. So the NACK flag is only
>> used for 'real' nack not protocol ones.
>>
>> Concerning the read issue, it have attached a patch (apply it on top of
>> Cyrille's patch). Could you have a try? I have reproduced your issue only one
>> time so it's hard for me to tell if it works well or not.
> 
> Since you removed the dev_err call, I don't really know if the underlying
> issue is solved or not (if there ever was any).
> 
> Reading RHR in the if at the top of at91_twi_read_next_byte() never happens
> for me (I added a witness and left it running overnight), so that
> seems like an unrelated change.

Ahhh, now I see. Why is it that I have to hit send before I start to realize
things? You have moved the RHR read from at91_do_twi_transfer()...

> Checking RXRDY before TXCOMP|NACK seems about equal, but what do I know?

...and there is no else 

Re: [PATCH v3 1/1] i2c: add ACPI support for I2C mux ports

[Dustin Byford]

On Wed Oct 21 11:12, Mika Westerberg wrote:
> On Tue, Oct 20, 2015 at 10:49:59AM -0700, Dustin Byford wrote:
> > I considered it, but I thought a default that fairly closely matches the
> > old behavior was more convenient.
> > 
> > On the other hand, leaving it up to the controllers makes it all very
> > explicit and perhaps simpler to reason about.
> > 
> > 
> > I could be convinced either way.  But, if we move it to the controller
> > drivers, which ones need the change?
> > 
> > grep -i acpi drivers/i2c/busses/i2c*
> > 
> > shows 18 drivers that might care.
> 
> I'm quite confident the designware I2C is enough for now. Intel uses it
> for all SoCs with LPSS and I think AMD has the same block for their I2C
> solution.

I certainly care about i801, ismt, and isch.  Doesn't this affect any
i2c controller with clients that may be enumerated through ACPI?

--Dustin
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 1/1] i2c: add ACPI support for I2C mux ports

[Dustin Byford]

On Wed Oct 21 11:34, Mika Westerberg wrote:
> On Wed, Oct 21, 2015 at 01:21:16AM -0700, Dustin Byford wrote:
> > On Wed Oct 21 11:12, Mika Westerberg wrote:
> > > On Tue, Oct 20, 2015 at 10:49:59AM -0700, Dustin Byford wrote:
> > > > I considered it, but I thought a default that fairly closely matches the
> > > > old behavior was more convenient.
> > > > 
> > > > On the other hand, leaving it up to the controllers makes it all very
> > > > explicit and perhaps simpler to reason about.
> > > > 
> > > > 
> > > > I could be convinced either way.  But, if we move it to the controller
> > > > drivers, which ones need the change?
> > > > 
> > > > grep -i acpi drivers/i2c/busses/i2c*
> > > > 
> > > > shows 18 drivers that might care.
> > > 
> > > I'm quite confident the designware I2C is enough for now. Intel uses it
> > > for all SoCs with LPSS and I think AMD has the same block for their I2C
> > > solution.
> > 
> > I certainly care about i801, ismt, and isch.  Doesn't this affect any
> > i2c controller with clients that may be enumerated through ACPI?
> 
> Yes, but so far I haven't seen any other devices being used for this
> than the I2C designware.
> 
> Which hardware you are testing this patch on?

I'm working with a number of x86-based network switch platforms.  Mostly
rangeley at the moment, but I'm sure others are in the works.  Have a
look at:

http://www.opencompute.org/wiki/Networking/SpecsAndDesigns

for examples.


My goal, hence the recent patches, is to help the network switch
industry move a lot of platform description into ACPI.  That means lots
of complicated I2C trees; switches are full of I2C devices.

--Dustin
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [GIT PULL] On-demand device probing

[Geert Uytterhoeven]

Hi Rafael,

On Wed, Oct 21, 2015 at 1:34 AM, Rafael J. Wysocki  wrote:
> On Tuesday, October 20, 2015 09:15:01 AM Rob Herring wrote:
>> On Tue, Oct 20, 2015 at 2:56 AM, Rafael J. Wysocki  
>> wrote:
>> > ACPI uses platform devices too.  In fact, ACPI device objects are 
>> > enumerated as
>> > platform devices by default now.
>>
>> Okay, I should have grepped for that:
>> drivers/base/platform.c:ACPI_COMPANION_SET(>dev, NULL);
>> drivers/base/platform.c:len = acpi_device_modalias(dev, buf,
>> PAGE_SIZE -1);
>> drivers/base/platform.c:rc = acpi_device_uevent_modalias(dev, env);
>> drivers/base/platform.c:/* Then try ACPI style match */
>> drivers/base/platform.c:if (acpi_driver_match_device(dev, drv))
>>
>> These are all cases which have DT version as well, so we're not really
>> all that different here. There's a few more for DT, but that probably
>> means you have just not hit the problems we have yet. For example,
>> what happens if you have an interrupt line in which the controller is
>> probed after the device connected to the interrupt line? That required
>> resolving irqs in platform_get_irq rather than using static resources
>> to support deferred probe.
>
> We don't have this particular problem, because the IRQ controllers are
> enumerated in a special way.

What does "in a special way" mean? Can you please be more specific?

Can you have interrupt controllers that depend on clocks, pin controllers,
and PM domains?

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html