Re: [MinnowBoard] Linux x86 I2C device probing with ACPI

[Mika Westerberg]

On Wed, Oct 21, 2015 at 09:19:24PM +0200, Christophe Ricard wrote:
> 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.

By probed do you mean that they are not listed under
/sys/bus/i2c/devices/ or that a driver is not probed against an existing
device?

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

If the device _STA() method returns 0 then we do not enumerate it. You
can check this by looking for the corresponding ACPI device node. For
example I have here I2C connected touch screen:

# cat /sys/bus/acpi/devices/NTRG0001\:00/status 
15

15 means that it is there (among other things). Also the device is then
available as I2C device here:

# ls -1 /sys/bus/i2c/devices/
i2c-0
i2c-1
i2c-2
i2c-NTRG0001:00

> 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

You don't need the last one.

> 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, 

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

[Mika Westerberg]

On Wed, Oct 21, 2015 at 05:21:46PM +0300, Jarkko Nikula wrote:
> 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 

Acked-by: Mika Westerberg 
--
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 0/4] Support multiplexed main SMBus interface on SB800

[Mika Westerberg]

On Tue, Oct 20, 2015 at 05:19:35PM +0200, Wolfram Sang wrote:
> On Tue, Aug 25, 2015 at 01:05:01PM +0200, Christian Fetzer wrote:
> > This is an attempt to upstream the patches created by Thomas Brandon and
> > Eddi De Pieri to support the multiplexed main SMBus interface on the SB800
> > chipset. 
> > (https://www.mail-archive.com/linux-i2c@vger.kernel.org/msg06757.html)
> > 
> > I have mainly rebased the latest patch version and tested the driver on a
> > HP ProLiant MicroServer G7 N54L (where this patch allows to access sensor 
> > data
> > from a w83795adg).
> > 
> > The patched driver is running stable on the machine, given that ic2_piix4 is
> > loaded before jc42 and w83795. If jc42 is loaded before i2c_piix4 calling
> > sensors triggers some errors:
> > ERROR: Can't get value of subfeature temp1_min_alarm: Can't read
> > 
> > While the kernel log shows:
> > i2c i2c-1: Transaction (pre): CNT=0c, CMD=05, ADD=31, DAT0=03, DAT1=c0
> > i2c i2c-1: Error: no response!
> > i2c i2c-1: Transaction (post): CNT=0c, CMD=05, ADD=31, DAT0=ff, DAT1=ff
> > Unfortunately I don't know how to tackle this specific issue.
> > 
> > Please review and let me know required changes in order to get this upstream
> > finally.
> > 
> > Eddi, Thomas, it would be great if you could verify the changes on your
> > machines.
> 
> Yes, additional tests are always good for a patch series
> 
> Asking the Intel guys for help, I have not much expierence with x86
> platforms... Mika, Jarkko, Andy any chance to help?

Unfortunately I don't have hardware this old to test 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

ACPI I2C device-driver matching issue

[Ben Gardner]

Hi all,

I have a custom Baytrail board with a M24C02 EEPROM attached to I2C bus 3.
I am using coreboot/SeaBIOS, so I have complete control over the ACPI tables.
I am using Linux 4.2.3.

I have defined a EEPROM device on I2C3 using I2cSerialBus() and it
shows up as expected.

Scope (\_SB.PCI0.I2C3) {
  Device (EEP0) {
Name (_CID, Package() { "24c02" })
Name (_CRS, ResourceTemplate () {
  I2cSerialBus (0x0057, ControllerInitiated, 40,
AddressingMode7Bit, "\\_SB.PCI0.I2C3", 0x00,
ResourceConsumer,,)
})
  }
}

Everything is nearly working, except that acpi_i2c_add_device() is
using the ACPI name to match the driver, which is "24C02:00".
The "at42" driver supports the device with the "24c02" alias.
i2c_match_id() in i2c-core.c uses strcmp() to match the device.
That obviously doesn't match, as "24c02" != "24C02:00".

When I modified acpi_i2c_add_device() to truncate at the colon and
convert it to lower case, it matches and works.


What is the right way to declare a I2C device in ACPI so that it
matches existing drivers?


For reference only, I included the change I made to get it to work below.
(Copy/pasted into gmail, so tabs are lost.)

Thanks,
Ben Gardner

diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index c83e4d1..64caddc 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -144,7 +144,13 @@ static acpi_status
acpi_i2c_add_device(acpi_handle handle, u32 level,
return AE_OK;

adev->power.flags.ignore_parent = true;
-   strlcpy(info.type, dev_name(>dev), sizeof(info.type));
+   {
+   const char *dn = dev_name(>dev);
+   int idx;
+   for (idx = 0; idx < sizeof(info.type) - 1 && dn[idx]
&& dn[idx] != ':'; idx++)
+   info.type[idx] = tolower(dn[idx]);
+   }
if (!i2c_new_device(adapter, )) {
adev->power.flags.ignore_parent = false;
dev_err(>dev,
--
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/22/2015 7:44 AM, Greg Kroah-Hartman wrote:
> 
> 
> On Thu, Oct 22, 2015 at 11:05:11AM +0200, Tomeu Vizoso wrote:
>> But that's moot currently because Greg believes that the time spent
>> probing devices at boot time could be reduced enough so that the order
>> in which devices are probed becomes irrelevant. IME that would have to
>> be under 200ms so that the user doesn't notice and that's unicorn-far
>> from any bootlog I have ever seen.
> 
> But as no one has actually produced a bootlog, how do you know that?
> Where exactly is your time being spent?  What driver is causing long
> delays?  Why is the long-delay-drivers not being done in their own
> thread?  And most importantly, why are you ignoring the work that people
> did back in 2008 to solve the issue on other hardware platforms?
> 
>> Given that downstreams are already carrying as many hacks as they
>> could think of to speed total boot up, I think this is effectively
>> telling them to go away.
> 
> No I'm not, I'm asking for real data, not hand-wavy-this-is-going-to
> solve-the-random-issue-i'm-having type patch by putting random calls in
> semi-random subsystems all over the kernel.
> 
> And when I ask for real data, you respond with the fact that you aren't
> trying to speed up boot time here at all, so what am I supposed to think

I also had the understanding that this patch series was about improving
boot time.  But I was kindly corrected that the behavior change was
getting the panel displaying stuff at an earlier point in the boot sequence,
_not_ completing the entire boot faster. 

The claim for the current series, in patch 0 in v7 is:

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

Just to get side-tracked, one other approach at ordering to reduce
deferrals reported a modest boot time reduction for four boards and a
very slight boot time increase for one other board.) The report of boot
times with that approach was in:

  http://article.gmane.org/gmane.linux.drivers.devicetree/133010

from Alexander Holler.

I have not searched further to see if there is more data of boot time
reductions from any of the other attempts to change driver binding
order to move dependencies before use of a resource.  But whether
there is a performance improvement or not, there continues to be
a stream of developers creatively impacting the binding order for
their specific driver(s) or board.  So it seems that maybe there
is an underlying problem, or we don't have adequate documentation
explaining how to avoid a need to order bindings, or the
documentation exists and is not being read.

I have been defaulting to the position that has been asserted by
the device tree maintainters, that probe deferrals work just fine
for at least the majority of cases (and is the message I have been
sharing in my conference presentations about device tree).  But I
suspect that there is at least a small minority of cases that are not
well served by probe deferral.  (Not to be read as an endorsement of
this specific patch series, just a generic observation.)

-Frank

> other than that you don't care enough to do the real work and are trying
> to hack the driver core up instead.
> 
>> Sorry for the rant,
> 
> No apologies needed, it's cathartic at times :)
> 
> thanks,
> 
> greg k-h
> .
> 

--
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

[Greg Kroah-Hartman]

On Thu, Oct 22, 2015 at 11:53:31AM -0700, Frank Rowand wrote:
> On 10/22/2015 7:44 AM, Greg Kroah-Hartman wrote:
> > 
> > 
> > On Thu, Oct 22, 2015 at 11:05:11AM +0200, Tomeu Vizoso wrote:
> >> But that's moot currently because Greg believes that the time spent
> >> probing devices at boot time could be reduced enough so that the order
> >> in which devices are probed becomes irrelevant. IME that would have to
> >> be under 200ms so that the user doesn't notice and that's unicorn-far
> >> from any bootlog I have ever seen.
> > 
> > But as no one has actually produced a bootlog, how do you know that?
> > Where exactly is your time being spent?  What driver is causing long
> > delays?  Why is the long-delay-drivers not being done in their own
> > thread?  And most importantly, why are you ignoring the work that people
> > did back in 2008 to solve the issue on other hardware platforms?
> > 
> >> Given that downstreams are already carrying as many hacks as they
> >> could think of to speed total boot up, I think this is effectively
> >> telling them to go away.
> > 
> > No I'm not, I'm asking for real data, not hand-wavy-this-is-going-to
> > solve-the-random-issue-i'm-having type patch by putting random calls in
> > semi-random subsystems all over the kernel.
> > 
> > And when I ask for real data, you respond with the fact that you aren't
> > trying to speed up boot time here at all, so what am I supposed to think
> 
> I also had the understanding that this patch series was about improving
> boot time.  But I was kindly corrected that the behavior change was
> getting the panel displaying stuff at an earlier point in the boot sequence,
> _not_ completing the entire boot faster. 
> 
> The claim for the current series, in patch 0 in v7 is:
> 
>With this series I get the kernel to output to the panel in 0.5s,
>instead of 2.8s.
> 
> Just to get side-tracked, one other approach at ordering to reduce
> deferrals reported a modest boot time reduction for four boards and a
> very slight boot time increase for one other board.) The report of boot
> times with that approach was in:
> 
>   http://article.gmane.org/gmane.linux.drivers.devicetree/133010
> 
> from Alexander Holler.
> 
> I have not searched further to see if there is more data of boot time
> reductions from any of the other attempts to change driver binding
> order to move dependencies before use of a resource.  But whether
> there is a performance improvement or not, there continues to be
> a stream of developers creatively impacting the binding order for
> their specific driver(s) or board.  So it seems that maybe there
> is an underlying problem, or we don't have adequate documentation
> explaining how to avoid a need to order bindings, or the
> documentation exists and is not being read.
> 
> I have been defaulting to the position that has been asserted by
> the device tree maintainters, that probe deferrals work just fine
> for at least the majority of cases (and is the message I have been
> sharing in my conference presentations about device tree).  But I
> suspect that there is at least a small minority of cases that are not
> well served by probe deferral.  (Not to be read as an endorsement of
> this specific patch series, just a generic observation.)

I agree, there might be some small numbers that this is a problem for,
and if so, great, show us the boot logs where things are taking up all
of the time, and we can work on resolving those issues.

But without hard numbers / details, this all is just random hand-waving,
and I don't like making core kernel changes on that basis.  And no one
else should ever want us to do that either.

thanks,

greg k-h
--
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

[Tomeu Vizoso]

On 22 October 2015 at 02:54, Rafael J. Wysocki  wrote:
> 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.

By "here" you mean what you are proposing for ordering device
suspends, or on-demand probing?

If you meant that probing on-demand is unneeded if we already have
dependency information, I agree with you and that's why I only pushed
forward on-demand, as the approach linked above introduced some
duplication when inferring the dependencies. Maybe that could be
avoided without too much refactoring.

In any case, Thierry mentioned the other day in #tegra that one could
also collect dependency information as a follow up to the on-demand
series by calling device_depend() or such instead of
of_device_probe().

Regards,

Tomeu

> Thanks,
> Rafael
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
--
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 v4 1/2] acpi: add acpi_preset_companion() stub

[Dustin Byford]

Add a stub for acpi_preset_companion().  Fixes build failures when
acpi_preset_companion() is used and CONFIG_ACPI is not set.

Signed-off-by: Dustin Byford 
---
 include/linux/acpi.h | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 51a96a8..66564f8 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -505,6 +505,12 @@ static inline bool has_acpi_companion(struct device *dev)
return false;
 }
 
+static inline void acpi_preset_companion(struct device *dev,
+struct acpi_device *parent, u64 addr)
+{
+   return;
+}
+
 static inline const char *acpi_dev_name(struct acpi_device *adev)
 {
return NULL;
-- 
2.1.4

--
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 22 October 2015 13:05:05 Wolfram Sang wrote:
> On Thu, Oct 22, 2015 at 02:10:52AM +0300, Laurent Pinchart wrote:
> > 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.
> 
> But HDMI worked on Koelsch in Dublin??

I know :-)

Do you have a Koelsch board now ? Could you try 
b9653e9c000dc2ebd9c8712442c659ccd1586e22 from Geert's drivers tree ? On my 
board the adv7511 fails to probe completely due to the regmap_read() failure.

-- 
Regards,

Laurent Pinchart

--
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 0/4] Support multiplexed main SMBus interface on SB800

[Mika Westerberg]

On Thu, Oct 22, 2015 at 01:03:25PM +0200, Wolfram Sang wrote:
> 
> > > > Please review and let me know required changes in order to get this 
> > > > upstream
> > > > finally.
> > > > 
> > > > Eddi, Thomas, it would be great if you could verify the changes on your
> > > > machines.
> > > 
> > > Yes, additional tests are always good for a patch series
> > > 
> > > Asking the Intel guys for help, I have not much expierence with x86
> > > platforms... Mika, Jarkko, Andy any chance to help?
> > 
> > Unfortunately I don't have hardware this old to test on :-/
> 
> And visual review? (That's what I need to do mostly, too)

Sure.

I don't have a copy of these patches but I went ahead and looked them up
from archives. Christian can you Cc me on next iteration?

Mostly they look good to me. Few comments though.

Patch 2/4: should we remove adapter in reverse order?

Patch 3/4: some stylistic issues, like:
- ERROR label should not be in capital letters actually it is
  not needed at all if you do unlock and return -EBUSY if
  request_region() fails.
- Block comment style

In addition I'm not sure if requesting io region for each transfer is
good idea. Can't we just request it once for this driver and handle the
necessary serialization using the mutex?
--
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 0/4] Support multiplexed main SMBus interface on SB800

[Wolfram Sang]

> > > Please review and let me know required changes in order to get this 
> > > upstream
> > > finally.
> > > 
> > > Eddi, Thomas, it would be great if you could verify the changes on your
> > > machines.
> > 
> > Yes, additional tests are always good for a patch series
> > 
> > Asking the Intel guys for help, I have not much expierence with x86
> > platforms... Mika, Jarkko, Andy any chance to help?
> 
> Unfortunately I don't have hardware this old to test on :-/

And visual review? (That's what I need to do mostly, too)



signature.asc
Description: Digital signature

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

[Wolfram Sang]

On Thu, Oct 22, 2015 at 02:10:52AM +0300, Laurent Pinchart wrote:
> 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.

But HDMI worked on Koelsch in Dublin??



signature.asc
Description: Digital signature

Re: [PATCH v2 1/1] i2c: core: fix a code to suppress a warning

[Andy Shevchenko]

On Fri, 2015-09-18 at 14:06 +0300, Andy Shevchenko wrote:
> There is a warning when compiling i2c-core.c
> drivers/i2c/i2c-core.c:2561:36: warning: dubious: x | !y
> 
> Fix it by using a plain bitwise AND since I2C_M_RD is a bit 0 and
> thus we are
> on the safe side.

Wolfram, as of today I didn't see this in linux-next. Should I amend
this one somehow?

> 
> Signed-off-by: Andy Shevchenko 
> ---
>  drivers/i2c/i2c-core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
> index 5f89f1e..a732107 100644
> --- a/drivers/i2c/i2c-core.c
> +++ b/drivers/i2c/i2c-core.c
> @@ -2555,7 +2555,7 @@ static u8 i2c_smbus_pec(u8 crc, u8 *p, size_t
> count)
>  static u8 i2c_smbus_msg_pec(u8 pec, struct i2c_msg *msg)
>  {
>   /* The address will be sent first */
> - u8 addr = (msg->addr << 1) | !!(msg->flags & I2C_M_RD);
> + u8 addr = (msg->addr << 1) | (msg->flags & I2C_M_RD);
>   pec = i2c_smbus_pec(pec, , 1);
>  
>   /* The data buffer follows */

-- 
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 1/3] i2c: uniphier: add UniPhier FIFO-less I2C driver

[Wolfram Sang]

On Thu, Jul 30, 2015 at 05:12:20PM +0900, Masahiro Yamada wrote:
> Add support for on-chip I2C controller used on old UniPhier SoCs
> such as PH1-LD4, PH1-sLD8, etc..  This adapter is so simple that
> it has no FIFO in it.
> 
> Signed-off-by: Masahiro Yamada 

Finally! Mostly looking good.

> +static u32 uniphier_i2c_functionality(struct i2c_adapter *adap)
> +{
> + return I2C_FUNC_I2C;
> +}

No I2C_FUNC_SMBUS_EMUL? But check if SMBUS_QUICK is supported by your
adapter.

> + regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + if (!regs) {
> + dev_err(dev, "failed to get memory resource");
> + return -EINVAL;
> + }

This 'if'-block can go. devm_ioremap_resource will check it for you.

Thanks,

   Wolfram



signature.asc
Description: Digital signature

Re: [PATCH 2/3] i2c: uniphier_f: add UniPhier FIFO-builtin I2C driver

[Wolfram Sang]

On Thu, Jul 30, 2015 at 05:12:21PM +0900, Masahiro Yamada wrote:
> Add support for on-chip I2C controller used on newer UniPhier SoCs
> such as PH1-Pro4, PH1-Pro5, etc.  This adapter is equipped with
> 8-depth TX/RX FIFOs.
> 
> Signed-off-by: Masahiro Yamada 

Same comments as for the other driver.

And quite some debug messages left. Could you check if you still want
them or if they were mostly useful during development of the driver?

Thanks,

   Wolfram



signature.asc
Description: Digital signature

Re: [PATCH 3/3] i2c: uniphier: add bindings for UniPhier I2C controllers

[Wolfram Sang]

On Thu, Jul 30, 2015 at 05:12:22PM +0900, Masahiro Yamada wrote:
> Device Tree bindings for two I2C controllers embedded in
> UniPhier SoCs.
> 
> Signed-off-by: Masahiro Yamada 

Please split this into two files with filenames matching those of the
drivers. I know they will be very similar but I'd like to keep
consistent.



signature.asc
Description: Digital signature

Re: [GIT PULL] On-demand device probing

[Mark Brown]

On Thu, Oct 22, 2015 at 04:02:13PM +0100, Russell King - ARM Linux wrote:

> If it was such a problem, then in the _eight_ days that this has been
> discussed so far, _someone_ would have sent some data showing the
> problem.  I think the fact is, there is no data.

> Someone prove me wrong.  Someone post the verifiable data showing that
> there is a problem to be solved here.

> Someone show what the specific failure cases are that are hampering
> vendors moving forwards.  Someone show the long boot times by way of
> kernel message log.  Someone show some evidence of the problems that
> have been alluded to.

> If no one can show some evidence, there isn't a problem here. :)

Yeah, I'm not convinced the timing is *such* a big deal either - I do
think that the log spam is a real problem but I think something much
less invasive like the interface you proposed is good for addressing
that.


signature.asc
Description: PGP signature

Re: Alternative approach to solve the deferred probe (was: [GIT PULL] On-demand device probing)

[Mark Brown]

On Tue, Oct 20, 2015 at 04:46:56PM +0100, Russell King - ARM Linux wrote:

> 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.

Yeah, plus I'd expect it to also result in better error reporting
overall if the subsystems are able to report when they fail to get
something rather than just returning an error to the driver.

> +/**
> + * dev_warn_deferred() - report why a probe has been deferred
> + */
> +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);

I'm not currently able to think of a nice way of writing this but I think
what I'd really like to see from a driver point of view is something
which decays into dev_err() if it's a non-deferral error.  That way
drivers can have minimal log and return error handling code and we will
still get the output sensibly.  The best I can think of is something
like

   void dev_warn_deferred(struct device *dev, int err, const char *fmt, ...)

which requires the caller to pass in err twice to get it logged.  That's
not a thing of beauty but it gets the job done...  but perhaps your
original interface is better, it's a bit cleaner.


signature.asc
Description: PGP signature

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

[Dustin Byford]

On Thu Oct 22 00:39, Rafael J. Wysocki wrote:
> 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.

I'm hoping an ack from Mika will suffice, but please let me know if
there's something I can do to verify or help ensure this.

--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

[Tomeu Vizoso]

On 21 October 2015 at 23:50, Frank Rowand  wrote:
> 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.

The important point that I have struggled to explain is that right now
for downstreams to influence the order in which devices are probed,
they have to carry a substantial amount of patches that cannot be ever
upstreamed. This fiddling with initcall levels and link order means
changing files that are very frequently changing, increasing the
amount of work when rebasing and increasing the probability of
regressions after a rebase.

This just adds up to other shortcomings of mainline and ends up with
the net result of vendors getting stuck with 3.4 kernels on SoCs that
start production in 2015. Another consequence is that vendors don't
have a chance to upstream their stuff even if they cared. The
overarching goal of the project I'm in is to reduce those shortcomings
that downstreams have to workaround, to facilitate their involvement
upstream.

With this series, the order in which devices are probed becomes the
order in which they were registered, which is the order in which the
devices appear in the FW description of the hw or in the board files
(much more predictable, which makes for a more robust process). For DT
and board files, which cover a good part of the consumer devices
shipped today with Linux, the downstream could just change the order
of device nodes and get their display or whatever to probe before any
other devices.

And even if downstream's hw has a SoC .dtsi that exists in mainline,
they could add a step to their build process that automatically
reorders the nodes to avoid carrying changes to that DT fragment.

But that's moot currently because Greg believes that the time spent
probing devices at boot time could be reduced enough so that the order
in which devices are probed becomes irrelevant. IME that would have to
be under 200ms so that the user doesn't notice and that's unicorn-far
from any bootlog I have ever seen.

Given that downstreams are already carrying as many hacks as they
could think of to speed total boot up, I think this is effectively
telling them to go away.

Sorry for the rant,

Tomeu

>> 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 

[PATCH v4 0/2] i2c: acpi: scan ACPI enumerated I2C mux channels

[Dustin Byford]

The following series adds support for describing ACPI enumerated I2C mux
ports like this (added as Documentation/acpi/i2c-muxes.txt):

+--+   +--+
| SMB1 |-->| MUX0 |--CH00--> i2c client A (0x50)
|  |   | 0x70 |--CH01--> i2c client B (0x50)
+--+   +--+

Device (SMB1)
{
Name (_HID, ...)
Device (MUX0)
{
Name (_HID, ...)
Name (_CRS, ResourceTemplate () {
I2cSerialBus (0x70, ControllerInitiated, I2C_SPEED,
  AddressingMode7Bit, "^SMB1", 0x00,
  ResourceConsumer,,)
}

Device (CH00)
{
Name (_ADR, 0)

Device (CLIA)
{
Name (_HID, ...)
Name (_CRS, ResourceTemplate () {
I2cSerialBus (0x50, ControllerInitiated, I2C_SPEED,
  AddressingMode7Bit, "^CH00", 0x00,
  ResourceConsumer,,)
}
}
}

Device (CH01)
{
Name (_ADR, 1)

Device (CLIB)
{
Name (_HID, ...)
Name (_CRS, ResourceTemplate () {
I2cSerialBus (0x50, ControllerInitiated, I2C_SPEED,
  AddressingMode7Bit, "^CH01", 0x00,
  ResourceConsumer,,)
}
}
}
}
}

v4:
- Moved the acpi_preset_companion() stub to a separate patch.
- Moved ACPI companion set from i2c-core to i801, ismt, and designware
  drivers.  With a minor rearrangement it was much easier to verify the
  drivers are all consistent (hopefully a little extra churn is warranted)

I was able to test i801 and ismt myself, but I could use some help making
sure the designware driver is OK since I don't have the hardware.

v3:
- Correct to and cc list (sorry git-send-email trouble again)

v2:
- Drop duplicate patch already submitted by Andy Shevchenko (i2c / ACPI:
  Rework I2C device scanning)
- Whitespace cleanup suggested by Mika
- Implement a acpi_preset_companion() stub for when CONFIG_ACPI is not set.
- Instead of special casing I2C muxes with regards to enumerating client
  devices, make sure adap->dev always has an ACPI companion.

I based this on linux-pm/bleeding-edge, but now it depends on Andy's change
(i2c / ACPI: Rework I2C device scanning) and I don't know where the rest of
his patch set is going.  Let me know if there's a more appropriate branch
and I'll be happy to rebase.

Thanks,

--Dustin

Dustin Byford (2):
  acpi: add acpi_preset_companion() stub
  i2c: add ACPI support for I2C mux ports

 Documentation/acpi/i2c-muxes.txt| 58 +
 drivers/i2c/busses/i2c-designware-pcidrv.c  |  1 +
 drivers/i2c/busses/i2c-designware-platdrv.c |  6 ++-
 drivers/i2c/busses/i2c-i801.c   |  9 ++---
 drivers/i2c/busses/i2c-ismt.c   |  8 +---
 drivers/i2c/i2c-core.c  |  4 +-
 drivers/i2c/i2c-mux.c   |  8 
 include/linux/acpi.h|  6 +++
 8 files changed, 84 insertions(+), 16 deletions(-)
 create mode 100644 Documentation/acpi/i2c-muxes.txt

-- 
2.1.4

--
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 v4 2/2] i2c: add ACPI support for I2C mux ports

[Dustin Byford]

Although I2C mux devices are easily enumerated using ACPI (_HID/_CID or
device property compatible string match), enumerating I2C client devices
connected through an I2C mux needs a little extra work.

This change implements a method for describing an I2C device hierarchy that
includes mux devices by using an ACPI Device() for each mux channel along
with an _ADR to set the channel number for the device.  See
Documentation/acpi/i2c-muxes.txt for a simple example.

To make this work the ismt, i801, and designware pci/platform devs now
share an ACPI companion with their I2C adapter dev similar to how it's done
in OF.  This is done on the assumption that power management functions will
not be called directly on the I2C dev that is sharing the ACPI node.

Signed-off-by: Dustin Byford 
---
 Documentation/acpi/i2c-muxes.txt| 58 +
 drivers/i2c/busses/i2c-designware-pcidrv.c  |  1 +
 drivers/i2c/busses/i2c-designware-platdrv.c |  6 ++-
 drivers/i2c/busses/i2c-i801.c   |  9 ++---
 drivers/i2c/busses/i2c-ismt.c   |  8 +---
 drivers/i2c/i2c-core.c  |  4 +-
 drivers/i2c/i2c-mux.c   |  8 
 7 files changed, 78 insertions(+), 16 deletions(-)
 create mode 100644 Documentation/acpi/i2c-muxes.txt

diff --git a/Documentation/acpi/i2c-muxes.txt b/Documentation/acpi/i2c-muxes.txt
new file mode 100644
index 000..9fcc4f0
--- /dev/null
+++ b/Documentation/acpi/i2c-muxes.txt
@@ -0,0 +1,58 @@
+ACPI I2C Muxes
+--
+
+Describing an I2C device hierarchy that includes I2C muxes requires an ACPI
+Device () scope per mux channel.
+
+Consider this topology:
+
++--+   +--+
+| SMB1 |-->| MUX0 |--CH00--> i2c client A (0x50)
+|  |   | 0x70 |--CH01--> i2c client B (0x50)
++--+   +--+
+
+which corresponds to the following ASL:
+
+Device (SMB1)
+{
+Name (_HID, ...)
+Device (MUX0)
+{
+Name (_HID, ...)
+Name (_CRS, ResourceTemplate () {
+I2cSerialBus (0x70, ControllerInitiated, I2C_SPEED,
+  AddressingMode7Bit, "^SMB1", 0x00,
+  ResourceConsumer,,)
+}
+
+Device (CH00)
+{
+Name (_ADR, 0)
+
+Device (CLIA)
+{
+Name (_HID, ...)
+Name (_CRS, ResourceTemplate () {
+I2cSerialBus (0x50, ControllerInitiated, I2C_SPEED,
+  AddressingMode7Bit, "^CH00", 0x00,
+  ResourceConsumer,,)
+}
+}
+}
+
+Device (CH01)
+{
+Name (_ADR, 1)
+
+Device (CLIB)
+{
+Name (_HID, ...)
+Name (_CRS, ResourceTemplate () {
+I2cSerialBus (0x50, ControllerInitiated, I2C_SPEED,
+  AddressingMode7Bit, "^CH01", 0x00,
+  ResourceConsumer,,)
+}
+}
+}
+}
+}
diff --git a/drivers/i2c/busses/i2c-designware-pcidrv.c 
b/drivers/i2c/busses/i2c-designware-pcidrv.c
index df23e8c..5b9dcdb 100644
--- a/drivers/i2c/busses/i2c-designware-pcidrv.c
+++ b/drivers/i2c/busses/i2c-designware-pcidrv.c
@@ -256,6 +256,7 @@ static int i2c_dw_pci_probe(struct pci_dev *pdev,
adap->class = 0;
adap->algo = _dw_algo;
adap->dev.parent = >dev;
+   ACPI_COMPANION_SET(>dev, ACPI_COMPANION(>dev));
adap->nr = controller->bus_num;
 
snprintf(adap->name, sizeof(adap->name), "i2c-designware-pci");
diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c 
b/drivers/i2c/busses/i2c-designware-platdrv.c
index 472b882..9efeac6 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -267,12 +267,14 @@ static int dw_i2c_probe(struct platform_device *pdev)
i2c_set_adapdata(adap, dev);
adap->owner = THIS_MODULE;
adap->class = I2C_CLASS_DEPRECATED;
-   strlcpy(adap->name, "Synopsys DesignWare I2C adapter",
-   sizeof(adap->name));
adap->algo = _dw_algo;
adap->dev.parent = >dev;
+   ACPI_COMPANION_SET(>dev, ACPI_COMPANION(>dev));
adap->dev.of_node = pdev->dev.of_node;
 
+   strlcpy(adap->name, "Synopsys DesignWare I2C adapter",
+   sizeof(adap->name));
+
if (dev->pm_runtime_disabled) {
pm_runtime_forbid(>dev);
} else {
diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
index eaef9bc..d4a6e77 100644
--- a/drivers/i2c/busses/i2c-i801.c
+++ b/drivers/i2c/busses/i2c-i801.c
@@ -1251,6 +1251,9 @@ static int i801_probe(struct pci_dev *dev, const struct 
pci_device_id *id)
priv->adapter.owner = THIS_MODULE;
priv->adapter.class = i801_get_adapter_class(priv);
priv->adapter.algo = _algorithm;
+   

[PATCH] i2c: i801: Add support for Intel Broxton

[Jarkko Nikula]

This patch adds the SMBUS PCI ID of Intel Broxton.

Signed-off-by: Jarkko Nikula 
---
This goes on top of Mika's "[PATCH] i2c: i801: Add support for Intel DNV":
http://marc.info/?l=linux-i2c=14447401042=2
---
 drivers/i2c/busses/i2c-i801.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
index 47c2ddf76264..d8219bc2ac4e 100644
--- a/drivers/i2c/busses/i2c-i801.c
+++ b/drivers/i2c/busses/i2c-i801.c
@@ -61,6 +61,7 @@
  * Sunrise Point-H (PCH)   0xa123  32  hardyes yes yes
  * Sunrise Point-LP (PCH)  0x9d23  32  hardyes yes yes
  * DNV (SOC)   0x19df  32  hardyes yes yes
+ * Broxton (SOC)   0x5ad4  32  hardyes yes yes
  *
  * Features supported by this driver:
  * Software PECno
@@ -204,6 +205,7 @@
 #define PCI_DEVICE_ID_INTEL_SUNRISEPOINT_H_SMBUS   0xa123
 #define PCI_DEVICE_ID_INTEL_SUNRISEPOINT_LP_SMBUS  0x9d23
 #define PCI_DEVICE_ID_INTEL_DNV_SMBUS  0x19df
+#define PCI_DEVICE_ID_INTEL_BROXTON_SMBUS  0x5ad4
 
 struct i801_mux_config {
char *gpio_chip;
@@ -866,6 +868,7 @@ static const struct pci_device_id i801_ids[] = {
{ PCI_DEVICE(PCI_VENDOR_ID_INTEL, 
PCI_DEVICE_ID_INTEL_SUNRISEPOINT_H_SMBUS) },
{ PCI_DEVICE(PCI_VENDOR_ID_INTEL, 
PCI_DEVICE_ID_INTEL_SUNRISEPOINT_LP_SMBUS) },
{ PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_DNV_SMBUS) },
+   { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_BROXTON_SMBUS) },
{ 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 v2 2/2] i2c: at91: manage unexpected RXRDY flag when starting a transfer

[Wolfram Sang]

On Wed, Oct 21, 2015 at 03:44:04PM +0200, Ludovic Desroches wrote:
> 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

SMATCH
drivers/i2c/busses/i2c-at91.c:602 at91_do_twi_transfer warn: unused return: sr 
= at91_twi_read()

drivers/i2c/busses/i2c-at91.c: In function 'at91_do_twi_transfer':
drivers/i2c/busses/i2c-at91.c:550:11: warning: variable 'sr' set but not used 
[-Wunused-but-set-variable]
  unsigned sr;
   ^


signature.asc
Description: Digital signature

Re: [GIT PULL] On-demand device probing

[Greg Kroah-Hartman]

On Thu, Oct 22, 2015 at 11:05:11AM +0200, Tomeu Vizoso wrote:
> On 21 October 2015 at 23:50, Frank Rowand  wrote:
> > 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.
> 
> The important point that I have struggled to explain is that right now
> for downstreams to influence the order in which devices are probed,
> they have to carry a substantial amount of patches that cannot be ever
> upstreamed. This fiddling with initcall levels and link order means
> changing files that are very frequently changing, increasing the
> amount of work when rebasing and increasing the probability of
> regressions after a rebase.
> 
> This just adds up to other shortcomings of mainline and ends up with
> the net result of vendors getting stuck with 3.4 kernels on SoCs that
> start production in 2015. Another consequence is that vendors don't
> have a chance to upstream their stuff even if they cared. The
> overarching goal of the project I'm in is to reduce those shortcomings
> that downstreams have to workaround, to facilitate their involvement
> upstream.

The init order of drivers has no influence at all on the ability for
companies to have their individual drivers merged upstream, please don't
be so dramatic about this.

Worst case, a vendor keeps a single patch to drivers/Makefile in their
tree that reorders things, yes it will get conflicts on every release,
but really, it's trivial to maintain if they wish to keep doing this
type of thing.

Again, it is _not_ the reason that we are living with 2million+ lines of
code in vendor kernels.

thanks,

greg k-h
--
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

[Greg Kroah-Hartman]

On Thu, Oct 22, 2015 at 11:05:11AM +0200, Tomeu Vizoso wrote:
> But that's moot currently because Greg believes that the time spent
> probing devices at boot time could be reduced enough so that the order
> in which devices are probed becomes irrelevant. IME that would have to
> be under 200ms so that the user doesn't notice and that's unicorn-far
> from any bootlog I have ever seen.

But as no one has actually produced a bootlog, how do you know that?
Where exactly is your time being spent?  What driver is causing long
delays?  Why is the long-delay-drivers not being done in their own
thread?  And most importantly, why are you ignoring the work that people
did back in 2008 to solve the issue on other hardware platforms?

> Given that downstreams are already carrying as many hacks as they
> could think of to speed total boot up, I think this is effectively
> telling them to go away.

No I'm not, I'm asking for real data, not hand-wavy-this-is-going-to
solve-the-random-issue-i'm-having type patch by putting random calls in
semi-random subsystems all over the kernel.

And when I ask for real data, you respond with the fact that you aren't
trying to speed up boot time here at all, so what am I supposed to think
other than that you don't care enough to do the real work and are trying
to hack the driver core up instead.

> Sorry for the rant,

No apologies needed, it's cathartic at times :)

thanks,

greg k-h
--
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 1/2] i2c: at91: fix write transfers by clearing pending interrupt first

[Wolfram Sang]

On Wed, Oct 21, 2015 at 03:44:03PM +0200, Ludovic Desroches wrote:
> 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

Applied to for-next, thanks!

I considered for-current, but really want this to sit a little in
for-next to make sure there are no regressions. It will go back via
stable.



signature.asc
Description: Digital signature

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

[Wolfram Sang]

On Wed, Oct 21, 2015 at 10:09:17AM +0300, Jarkko Nikula wrote:
> 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 

Applied to for-next, thanks!



signature.asc
Description: Digital signature

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

[Wolfram Sang]

On Wed, Oct 21, 2015 at 05:21:46PM +0300, Jarkko Nikula wrote:
> 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 

Applied to for-next, thanks!



signature.asc
Description: Digital signature

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

[Wolfram Sang]

On Tue, Oct 20, 2015 at 04:32:24PM +0200, Thomas Petazzoni wrote:
> From: Hezi 
> 
> Commit 00d8689b85a7 ("i2c: mv64xxx: rework offload support to fix
> several problems") completely reworked the offload support, but
> stupidly left a debugging-related "return false" at the beginning of
> the mv64xxx_i2c_can_offload() function. This has the unfortunate
> consequence that offloading is in fact never used, which wasn't really
> the intention.
> 
> This commit fixes that problem by removing the stupid "return false".
> 
> Fixes: 00d8689b85a7 ("i2c: mv64xxx: rework offload support to fix several 
> problems")
> Cc: 
> Signed-off-by: Hezi 
> [Thomas: reworked commit log and title.]
> Signed-off-by: Thomas Petazzoni 

Applied to for-current, thanks!



signature.asc
Description: Digital signature

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

[Ken Xue]

On Wed, 2015-10-21 at 13:46 +0300, Mika Westerberg wrote:
> 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...

You are right :-). The new patch for reverting a445900c906092 is
submitted.
Please help to review. 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

[PATCH 1/1] i2c: designware: reverts "i2c: designware: Add support for AMD I2C controller"

[Ken Xue]

The patch reverts commit a445900c9060 (i2c: designware: Add support for
AMD I2C controller)

Since kernel starts to support APD(drivers/acpi/acpi_apd.c), there is
no need to get freq from id->driver_data for AMD0010. clkdev is supposed
to be already registered in APD.

So, revert old design and make AMD0010 looks like other ones.

Signed-off-by: Ken Xue 
Signed-off-by: Xiangliang Yu 
---
 drivers/i2c/busses/i2c-designware-platdrv.c | 27 +--
 1 file changed, 1 insertion(+), 26 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c 
b/drivers/i2c/busses/i2c-designware-platdrv.c
index 472b882..4bf5fc1 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -97,7 +97,6 @@ static void dw_i2c_acpi_params(struct platform_device *pdev, 
char method[],
 static int dw_i2c_acpi_configure(struct platform_device *pdev)
 {
struct dw_i2c_dev *dev = platform_get_drvdata(pdev);
-   const struct acpi_device_id *id;
 
dev->adapter.nr = -1;
dev->tx_fifo_depth = 32;
@@ -111,29 +110,9 @@ static int dw_i2c_acpi_configure(struct platform_device 
*pdev)
dw_i2c_acpi_params(pdev, "FMCN", >fs_hcnt, >fs_lcnt,
   >sda_hold_time);
 
-   /*
-* Provide a way for Designware I2C host controllers that are not
-* based on Intel LPSS to specify their input clock frequency via
-* id->driver_data.
-*/
-   id = acpi_match_device(pdev->dev.driver->acpi_match_table, >dev);
-   if (id && id->driver_data)
-   clk_register_fixed_rate(>dev, dev_name(>dev), NULL,
-   CLK_IS_ROOT, id->driver_data);
-
return 0;
 }
 
-static void dw_i2c_acpi_unconfigure(struct platform_device *pdev)
-{
-   struct dw_i2c_dev *dev = platform_get_drvdata(pdev);
-   const struct acpi_device_id *id;
-
-   id = acpi_match_device(pdev->dev.driver->acpi_match_table, >dev);
-   if (id && id->driver_data)
-   clk_unregister(dev->clk);
-}
-
 static const struct acpi_device_id dw_i2c_acpi_match[] = {
{ "INT33C2", 0 },
{ "INT33C3", 0 },
@@ -141,7 +120,7 @@ static const struct acpi_device_id dw_i2c_acpi_match[] = {
{ "INT3433", 0 },
{ "80860F41", 0 },
{ "808622C1", 0 },
-   { "AMD0010", 133 * 1000 * 1000 },
+   { "AMD0010", 0 },
{ }
 };
 MODULE_DEVICE_TABLE(acpi, dw_i2c_acpi_match);
@@ -150,7 +129,6 @@ static inline int dw_i2c_acpi_configure(struct 
platform_device *pdev)
 {
return -ENODEV;
 }
-static inline void dw_i2c_acpi_unconfigure(struct platform_device *pdev) { }
 #endif
 
 static int dw_i2c_probe(struct platform_device *pdev)
@@ -306,9 +284,6 @@ static int dw_i2c_remove(struct platform_device *pdev)
pm_runtime_put_sync(>dev);
pm_runtime_disable(>dev);
 
-   if (has_acpi_companion(>dev))
-   dw_i2c_acpi_unconfigure(pdev);
-
return 0;
 }
 
-- 
1.9.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: [GIT PULL] On-demand device probing

[Russell King - ARM Linux]

On Thu, Oct 22, 2015 at 07:44:05AM -0700, Greg Kroah-Hartman wrote:
> 
> 
> On Thu, Oct 22, 2015 at 11:05:11AM +0200, Tomeu Vizoso wrote:
> > Given that downstreams are already carrying as many hacks as they
> > could think of to speed total boot up, I think this is effectively
> > telling them to go away.
> 
> No I'm not, I'm asking for real data, not hand-wavy-this-is-going-to
> solve-the-random-issue-i'm-having type patch by putting random calls in
> semi-random subsystems all over the kernel.

+1000, fully agree.

There's too much verbal diarrhoea going on in this thread and no facts.
I've been waiting for real data too, and there's not one shred of it, or
even a hint that it might appear.  So, the conclusion I'm coming to is
that there isn't any data to back up the claims made in this thread.

If it was such a problem, then in the _eight_ days that this has been
discussed so far, _someone_ would have sent some data showing the
problem.  I think the fact is, there is no data.

Someone prove me wrong.  Someone post the verifiable data showing that
there is a problem to be solved here.

Someone show what the specific failure cases are that are hampering
vendors moving forwards.  Someone show the long boot times by way of
kernel message log.  Someone show some evidence of the problems that
have been alluded to.

If no one can show some evidence, there isn't a problem here. :)

-- 
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]

Hi Russell,

On 10/21/2015 09:28 PM, Russell King - ARM Linux wrote:
> 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.

Sorry Master, but you really don't need to spend so much time typing the
same things three times  - I understand what are you trying to do :(

I did my comments with assumption that it's not officially prohibited/deprecated
to register drivers (and execute probes) from late_initcall() layer
(just recommended) and there are still bunch of drivers which are doing this.
Now I see that it's not a recommendation any more, and deferred_probe_initcall()
might be a good place to activate driver_deferred_probe_report if goal is to
identify and fix such drivers.

Sorry for your time.

> 
>>
>> 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?
> 

Also, I've spend some time today testing your proposal - hope you'll find 
results
useful.

I've applied truncated version of your patch (diff below) on TI's 4.1 kernel and
run few tests (log is below) on dra7-evm/am43xx-gpevm - K4.1 is not far away 
from LKML,
so I assume this test is valid. Overall boot process consists from two stages:
kernel boot and modules loading. 

My Changes:
 - only really_probe() modified to show deferred device/drivers 

>From the log I can see additional messages in log when modules are loading,
because driver_deferred_probe_report is still true - dwc3 probes were deferred,
but then finally succeeded.

So, as you've mentioned, it seems a good thing to deactivate 
driver_deferred_probe_report and
provide user with ability to turn it on again (and probably re-trigger deferred
device probing).

I've found no issues during Kernel boot (built-in) time, new messages are 
displayed only
if probe is failed for some drivers:
[3.219700] == deferred_probe_initcalll
[3.226820] platform omapdrm.0: Driver omapdrm requests probe deferral
[3.233378] platform omapdrm.0: deferring probe:  Driver omapdrm 
requests probe deferral
[3.242084] dra7evm-tpd12s015 encoder@1: failed to parse CT CP HPD gpio
[3.248737] platform encoder@1: Driver dra7evm-tpd12s015 requests probe 
deferral
[3.256168] platform encoder@1: deferring probe:  Driver 
dra7evm-tpd12s015 requests probe deferral
[3.265763] connector-hdmi connector@1: failed to find video source

[PATCH] i2c: i2c-imx: Use -ENXIO as error in the NACK case

[Fabio Estevam]

According to Documentation/i2c/fault-codes the response to a bus NACK
should be -ENXIO, so fix the error code.

This change is similar to commit 6ff4b1051632 ("i2c: rcar: fix NACK
error code").

Signed-off-by: Fabio Estevam 
---
 drivers/i2c/busses/i2c-imx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c
index 785aa67..329cf13 100644
--- a/drivers/i2c/busses/i2c-imx.c
+++ b/drivers/i2c/busses/i2c-imx.c
@@ -461,7 +461,7 @@ static int i2c_imx_acked(struct imx_i2c_struct *i2c_imx)
 {
if (imx_i2c_read_reg(i2c_imx, IMX_I2C_I2SR) & I2SR_RXAK) {
dev_dbg(_imx->adapter.dev, "<%s> No ACK\n", __func__);
-   return -EIO;  /* No ACK */
+   return -ENXIO;  /* No ACK */
}
 
dev_dbg(_imx->adapter.dev, "<%s> ACK received\n", __func__);
-- 
1.9.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