* RESEND:v3 i2c-eg20t:fix race between i2c init and interrupt enable
From: Yadi Hu @ 2016-09-22 1:09 UTC (permalink / raw)
To: yadi.hu, wsa, jdelvare; +Cc: linux-i2c
the eg20t driver call request_irq() function before the pch_base_address,
base address of i2c controller's register, isassigned an effective value.
there is one possible scenario that an interrupt which isn't inside eg20t
arrives immediately after request_irq() is executed when i2c controller
shares an interrupt number with others. since the interrupt handler
pch_i2c_handler() has already active as shared action, it will be called
and read its own register to determine if this interrupt is from itself.
At that moment, since base address of i2c registers is not remapped
in kernel space yet,so the INT handler will access an illegal address
and then a error occurs.
Bad IO access at port 0x18 (return inl(port))
Call Trace:
[<c102c733>] warn_slowpath_common+0x73/0xa0
[<c13109f5>] ? bad_io_access+0x45/0x50
[<c13109f5>] ? bad_io_access+0x45/0x50
[<c102c804>] warn_slowpath_fmt+0x34/0x40
[<c13109f5>] bad_io_access+0x45/0x50
[<c1310b62>] ioread32+0x22/0x40
[<c14c60db>] pch_i2c_handler+0x3b/0x120
[<c1092f34>] handle_irq_event_percpu+0x64/0x330
[<c109323b>] handle_irq_event+0x3b/0x60
[<c1095b50>] ? unmask_irq+0x30/0x30
[<c1095ba0>] handle_fasteoi_irq+0x50/0xe0
<IRQ> [<c16e7e78>] ? do_IRQ+0x48/0xc0
[<c16e7f6f>] ? smp_apic_timer_interrupt+0x7f/0x17a
[<c16e7da9>] ? common_interrupt+0x29/0x30
[<c1008ebb>] ? mwait_idle+0x8b/0x2c0
[<c1009e8e>] ? cpu_idle+0x9e/0xc0
[<c16be408>] ? rest_init+0x6c/0x74
[<c19f770a>] ? start_kernel+0x311/0x318
[<c19f7231>] ? repair_env_string+0x51/0x51
[<c19f7078>] ? i386_start_kernel+0x78/0x7d
---[ end trace eb3a1028f468a140 ]---
i2c_eg20t 0000:05:0c.2: pch_i2c_handler :I2C-3 mode(0) is not supported
Yadi Hu (1)
i2c-eg20t: fix race between i2c init and interrupt enable
drivers/i2c/busses/i2c-eg20t.c | 18 +++++++++++-------
1 file changed, 11 insertions(+), 7 deletions(-)
^ permalink raw reply
* Re: [PATCH] i2c-eg20t: fix race between i2c init and interrupt enable
From: Yadi @ 2016-09-22 1:04 UTC (permalink / raw)
To: Wolfram Sang; +Cc: jdelvare, linux-i2c
In-Reply-To: <20160921161608.GB1432@katana>
On 2016年09月22日 00:16, Wolfram Sang wrote:
> On Sun, Sep 18, 2016 at 06:52:31PM +0800, Yadi Hu wrote:
>> From: "Yadi.hu" <yadi.hu@windriver.com>
>>
>> the eg20t driver call request_irq() function before the pch_base_address,
>> base address of i2c controller's register, is assigned an effective value.
>>
>> there is one possible scenario that an interrupt which isn't inside eg20t
>> arrives immediately after request_irq() is executed when i2c controller
>> shares an interrupt number with others. since the interrupt handler
>> pch_i2c_handler() has already active as shared action, it will be called
>> and read its own register to determine if this interrupt is from itself.
>>
>> At that moment, since base address of i2c registers is not remapped
>> in kernel space yet,so the INT handler will access an illegal address
>> and then a error occurs.
>>
>> Signed-off-by: Yadi.hu <yadi.hu@windriver.com>
> Applied to for-next, thanks!
>
> Please make sure "V3" also appears in the patch subject since patch
> management tools pick this up. "-v <nr>" in recent git versions makes
> this super easy.
Got it, I will resend the patch with V3 tag.
Yadi
^ permalink raw reply
* Re: i2c: xlp9xx: ACPI support for I2C clients
From: Wolfram Sang @ 2016-09-21 21:17 UTC (permalink / raw)
To: Tanmay Jagdale; +Cc: linux-i2c, jayachandran.chandrashekaran
In-Reply-To: <1473967659-9000-1-git-send-email-tanmay.jagdale@broadcom.com>
[-- Attachment #1: Type: text/plain, Size: 124 bytes --]
> + ACPI_COMPANION_SET(&(priv->adapter.dev), ACPI_COMPANION(&pdev->dev));
No need for parenthesis in the first argument.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH] i2c: i2c-mux-pca954x: retry updating the mux selection on failure
From: Wolfram Sang @ 2016-09-21 21:11 UTC (permalink / raw)
To: Peter Rosin; +Cc: open list:I2C MUXES, open list
In-Reply-To: <1473859452-8069-1-git-send-email-peda@axentia.se>
[-- Attachment #1: Type: text/plain, Size: 313 bytes --]
On Wed, Sep 14, 2016 at 03:24:12PM +0200, Peter Rosin wrote:
> The cached value of the last selected channel prevents retries on the
> next call, even on failure to update the selected channel. Fix that.
>
> Signed-off-by: Peter Rosin <peda@axentia.se>
Applied to for-current and added stable, thanks!
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH 5/5] i2c: octeon,thunderx: Limit register access retries
From: Wolfram Sang @ 2016-09-21 21:03 UTC (permalink / raw)
To: Jan Glauber; +Cc: linux-kernel, linux-i2c
In-Reply-To: <a52c3157fd27c939185163d2df23e39174cabc60.1474439371.git.jglauber@cavium.com>
[-- Attachment #1: Type: text/plain, Size: 490 bytes --]
On Wed, Sep 21, 2016 at 08:51:06AM +0200, Jan Glauber wrote:
> Do not infinitely retry register readq and writeq operations
> in order to not lock up the CPU in case the TWSI gets stuck.
>
> Return -EIO in case of a failed data read. For all other
> cases just return so subsequent operations will fail
> and trigger the recovery.
>
> Signed-off-by: Jan Glauber <jglauber@cavium.com>
I didn't really check, but have you considered using
readq_poll_timeout() from iopoll.h?
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH 3/5] i2c: octeon,thunderx: Fix high-level controller status check
From: Wolfram Sang @ 2016-09-21 21:01 UTC (permalink / raw)
To: Jan Glauber; +Cc: linux-kernel, linux-i2c
In-Reply-To: <3d2e2e4e91e2245318a278b19f22e9b1af3204d1.1474439371.git.jglauber@cavium.com>
[-- Attachment #1: Type: text/plain, Size: 414 bytes --]
On Wed, Sep 21, 2016 at 08:51:04AM +0200, Jan Glauber wrote:
> In case the high-level controller (HLC) is used the status code is
> reported at a different location. Check that location after HLC
> write operations if the ready bit is not set and return an appropriate
> error code instead of always returning -EAGAIN.
>
> Signed-off-by: Jan Glauber <jglauber@cavium.com>
Applied to for-next, thanks!
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH 2/5] i2c: octeon,thunderx: Avoid sending STOP during recovery
From: Wolfram Sang @ 2016-09-21 21:00 UTC (permalink / raw)
To: Jan Glauber; +Cc: linux-kernel, linux-i2c, Dmitry Bazhenov
In-Reply-To: <9791f6fb5fba70ddf9bd6b92e113e97bc0483317.1474439371.git.jglauber@cavium.com>
[-- Attachment #1: Type: text/plain, Size: 610 bytes --]
On Wed, Sep 21, 2016 at 08:51:03AM +0200, Jan Glauber wrote:
> From: Dmitry Bazhenov <dmitry.bazhenov@auriga.com>
>
> Due to a bug in the ThunderX I2C hardware sending STOP during
> a recovery attempt could lock up the hardware. To work around
> this problem do not send STOP at the beginning of the recovery
> but use the override registers to bring the TWSI including
> the high-level controller out of the bad state.
>
> Signed-off-by: Dmitry Bazhenov <dmitry.bazhenov@auriga.com>
> Signed-off-by: Jan Glauber <jglauber@cavium.com>
> [Changed commit message]
Applied to for-next, thanks!
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH 1/5] i2c: octeon,thunderx: Fix set SCL recovery function
From: Wolfram Sang @ 2016-09-21 21:00 UTC (permalink / raw)
To: Jan Glauber; +Cc: linux-kernel, linux-i2c, Dmitry Bazhenov
In-Reply-To: <cea4dddccc4f24d90c3783159c5236d6124facf0.1474439371.git.jglauber@cavium.com>
[-- Attachment #1: Type: text/plain, Size: 495 bytes --]
On Wed, Sep 21, 2016 at 08:51:02AM +0200, Jan Glauber wrote:
> From: Dmitry Bazhenov <dmitry.bazhenov@auriga.com>
>
> The set SCL recovery function unconditionally pulls the SCL line low.
> Only pull SCL line low according to val parameter.
>
> Signed-off-by: Dmitry Bazhenov <dmitry.bazhenov@auriga.com>
> Signed-off-by: Jan Glauber <jglauber@cavium.com>
> [Changed commit message]
Applied to for-next, thanks!
Minor nit: please only use "octeon:" if you change octeon-core.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH 4/5] i2c: octeon,thunderx: Check bus state before starting a transaction
From: Wolfram Sang @ 2016-09-21 20:55 UTC (permalink / raw)
To: Jan Glauber; +Cc: linux-kernel, linux-i2c
In-Reply-To: <21b1db0af4c62a53dc2fcec1e0b1f7b412e6f1be.1474439371.git.jglauber@cavium.com>
[-- Attachment #1: Type: text/plain, Size: 287 bytes --]
On Wed, Sep 21, 2016 at 08:51:05AM +0200, Jan Glauber wrote:
> Add an additional status check before starting a transaction and,
> if required, trigger the recovery if the check fails.
>
> Signed-off-by: Jan Glauber <jglauber@cavium.com>
Won't this break multi-master setups?
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH] i2c-eg20t: fix race between i2c init and interrupt enable
From: Wolfram Sang @ 2016-09-21 16:18 UTC (permalink / raw)
To: Yadi Hu; +Cc: jdelvare, linux-i2c
In-Reply-To: <20160921161608.GB1432@katana>
[-- Attachment #1: Type: text/plain, Size: 1040 bytes --]
On Wed, Sep 21, 2016 at 06:16:09PM +0200, Wolfram Sang wrote:
> On Sun, Sep 18, 2016 at 06:52:31PM +0800, Yadi Hu wrote:
> > From: "Yadi.hu" <yadi.hu@windriver.com>
> >
> > the eg20t driver call request_irq() function before the pch_base_address,
> > base address of i2c controller's register, is assigned an effective value.
> >
> > there is one possible scenario that an interrupt which isn't inside eg20t
> > arrives immediately after request_irq() is executed when i2c controller
> > shares an interrupt number with others. since the interrupt handler
> > pch_i2c_handler() has already active as shared action, it will be called
> > and read its own register to determine if this interrupt is from itself.
> >
> > At that moment, since base address of i2c registers is not remapped
> > in kernel space yet,so the INT handler will access an illegal address
> > and then a error occurs.
> >
> > Signed-off-by: Yadi.hu <yadi.hu@windriver.com>
>
> Applied to for-next, thanks!
I meant: applied to for-current!
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH] i2c-eg20t: fix race between i2c init and interrupt enable
From: Wolfram Sang @ 2016-09-21 16:16 UTC (permalink / raw)
To: Yadi Hu; +Cc: jdelvare, linux-i2c
In-Reply-To: <1474195951-7238-2-git-send-email-yadi.hu@windriver.com>
[-- Attachment #1: Type: text/plain, Size: 1063 bytes --]
On Sun, Sep 18, 2016 at 06:52:31PM +0800, Yadi Hu wrote:
> From: "Yadi.hu" <yadi.hu@windriver.com>
>
> the eg20t driver call request_irq() function before the pch_base_address,
> base address of i2c controller's register, is assigned an effective value.
>
> there is one possible scenario that an interrupt which isn't inside eg20t
> arrives immediately after request_irq() is executed when i2c controller
> shares an interrupt number with others. since the interrupt handler
> pch_i2c_handler() has already active as shared action, it will be called
> and read its own register to determine if this interrupt is from itself.
>
> At that moment, since base address of i2c registers is not remapped
> in kernel space yet,so the INT handler will access an illegal address
> and then a error occurs.
>
> Signed-off-by: Yadi.hu <yadi.hu@windriver.com>
Applied to for-next, thanks!
Please make sure "V3" also appears in the patch subject since patch
management tools pick this up. "-v <nr>" in recent git versions makes
this super easy.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH v2] i2c / ACPI: Do not touch an I2C device if it belongs to another adapter
From: Wolfram Sang @ 2016-09-21 16:14 UTC (permalink / raw)
To: Mika Westerberg
Cc: Nicolai Stange, Octavian Purdila, Rafael J . Wysocki,
Jarkko Nikula, linux-i2c, linux-kernel
In-Reply-To: <20160921084502.GV1811@lahna.fi.intel.com>
[-- Attachment #1: Type: text/plain, Size: 1823 bytes --]
On Wed, Sep 21, 2016 at 11:45:02AM +0300, Mika Westerberg wrote:
> On Wed, Sep 21, 2016 at 07:48:35AM +0200, Wolfram Sang wrote:
> > On Tue, Sep 20, 2016 at 04:59:25PM +0300, Mika Westerberg wrote:
> > > When enumerating I2C devices connected to an I2C adapter we scan the whole
> > > namespace (as it is possible to have devices anywhere in that namespace,
> > > not just below the I2C adapter device) and add each found device to the I2C
> > > bus in question.
> > >
> > > Now after commit 525e6fabeae2 ("i2c / ACPI: add support for ACPI
> > > reconfigure notifications") checking of the adapter handle to the one found
> > > in the I2cSerialBus() resource was moved to happen after resources of the
> > > I2C device has been parsed. This means that if the I2cSerialBus() resource
> > > points to an adapter that does not exists in the system we still parse
> > > those resources. This is problematic in particular because
> > > acpi_dev_resource_interrupt() tries to configure GSI if the device also has
> > > an Interrupt() resource. Failing to do that results errrors like this to be
> > > printed on the console:
> > >
> > > [ 10.409490] ERROR: Unable to locate IOAPIC for GSI 37
> > >
> > > To fix this we pass the I2C adapter to i2c_acpi_get_info() and make sure
> > > the handle matches the one in the I2cSerialBus() resource before doing
> > > anything else to the device.
> > >
> > > Reported-and-tested-by: Nicolai Stange <nicstange@gmail.com>
> > > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> >
> > Considering this for for-current. So shall we add:
> >
> > Fixes: 525e6fabeae2 ("i2c / ACPI: add support for ACPI reconfigure notifications")
> >
> > ?
>
> Yes please :)
Huh? It doesn't apply on top of rc7 here? What did you base it on?
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH 2/3] i2c: bcm2835: Add support for combined write-read transfer
From: Noralf Trønnes @ 2016-09-21 13:45 UTC (permalink / raw)
To: kernel
Cc: wsa, Stephen Warren, Eric Anholt, linux-rpi-kernel, linux-kernel,
linux-arm-kernel, linux-i2c
In-Reply-To: <DF457EB8-D1B8-4BA4-B79C-AF01AF7CAA3B@martin.sperl.org>
Den 20.09.2016 13:29, skrev kernel@martin.sperl.org:
>> On 20.09.2016, at 12:56, Noralf Trønnes <noralf@tronnes.org> wrote:
>>
>>
>> Den 20.09.2016 12:15, skrev Martin Sperl:
>>>
>>> On 20.09.2016 10:41, Noralf Trønnes wrote:
>>>> Den 20.09.2016 09:19, skrev Martin Sperl:
>>>>> Hi Noralf!
>>>>>
>>>>> On 19.09.2016 17:26, Noralf Trønnes wrote:
>>>>>> Some SMBus protocols use Repeated Start Condition to switch from write
>>>>>> mode to read mode. Devices like MMA8451 won't work without it.
>>>>>>
>>>>>> When downstream implemented support for this in i2c-bcm2708, it broke
>>>>>> support for some devices, so a module parameter was added and combined
>>>>>> transfer was disabled by default.
>>>>>> See https://github.com/raspberrypi/linux/issues/599
>>>>>> It doesn't seem to have been any investigation into what the problem
>>>>>> really was. Later there was added a timeout on the polling loop.
>>>>>>
>>>>>> One of the devices mentioned to partially stop working was DS1307.
>>>>>>
>>>>>> I have run thousands of transfers to a DS1307 (rtc), MMA8451 (accel)
>>>>>> and AT24C32 (eeprom) in parallel without problems.
>>>>>>
>>>>>> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
>>>>>> ---
>>>>>> drivers/i2c/busses/i2c-bcm2835.c | 107
>>>>>> +++++++++++++++++++++++++++++++++++----
>>>>>> 1 file changed, 98 insertions(+), 9 deletions(-)
>>>>> ...
>>>>>> @@ -209,8 +289,17 @@ static int bcm2835_i2c_xfer(struct i2c_adapter
>>>>>> *adap, struct i2c_msg msgs[],
>>>>>> int i;
>>>>>> int ret = 0;
>>>>>> + /* Combined write-read to the same address (smbus) */
>>>>>> + if (num == 2 && (msgs[0].addr == msgs[1].addr) &&
>>>>>> + !(msgs[0].flags & I2C_M_RD) && (msgs[1].flags & I2C_M_RD) &&
>>>>>> + (msgs[0].len <= 16)) {
>>>>>> + ret = bcm2835_i2c_xfer_msg(i2c_dev, &msgs[0], &msgs[1]);
>>>>>> +
>>>>>> + return ret ? ret : 2;
>>>>>> + }
>>>>>> +
>>>>>> for (i = 0; i < num; i++) {
>>>>>> - ret = bcm2835_i2c_xfer_msg(i2c_dev, &msgs[i]);
>>>>>> + ret = bcm2835_i2c_xfer_msg(i2c_dev, &msgs[i], NULL);
>>>>>> if (ret)
>>>>>> break;
>>>>>> }
>>>>> This does not seem to implement the i2c_msg api correctly.
>>>>>
>>>>> As per comments in include/uapi/linux/i2c.h on line 58 only the last
>>>>> message
>>>>> in a group should - by default - send a STOP.
>>>>>
>>>> Apparently it's a known problem that the i2c controller doesn't support
>>>> Repeated Start. It will always issue a Stop when it has transferred DLEN
>>>> bytes.
>>>> Refs:
>>>> http://www.circuitwizard.de/raspi-i2c-fix/raspi-i2c-fix.html
>>>> http://raspberrypi.stackexchange.com/questions/31728/has-anyone-successfully-used-i2c-repeated-starts-on-the-pi2-my-scope-says-they
>>>>
>>>>
>>>> UNLESS: a Start Transfer (ST) is issued after Transfer Active (TA) is set
>>>> and before DONE is set (or the last byte is shifted, I don't know excatly).
>>>> Refs:
>>>> https://github.com/raspberrypi/linux/issues/254#issuecomment-15254134
>>>> https://www.raspberrypi.org/forums/viewtopic.php?p=807834&sid=2b612c7209f2175bf1a266359c72ae6c#p807834
>>>>
>>>>
>>>> I found this answer/report by joan that the downstream combined support
>>>> isn't reliable:
>>>> http://raspberrypi.stackexchange.com/questions/31728/has-anyone-successfully-used-i2c-repeated-starts-on-the-pi2-my-scope-says-they
>>>>
>>>>
>>>> My implementation differs from downstream in that I use local_irq_save()
>>>> to protect the polling loop. But that only protects from missing the TA
>>>> (downstream can miss the TA and issue a Stop).
>>>>
>>>> So currently in mainline we have a driver that says it support the standard
>>>> (I2C_FUNC_I2C), but it really only supports one message transfers since it
>>>> can't do ReStart.
>>>>
>>>> What I have done in this patch is to support ReStart for transfers with
>>>> 2 messages: first write, then read. But maybe a better solution is to just
>>>> leave this alone if it is flaky and use bitbanging instead. I don't know.
>>> I have not said that the approach you have taken is wrong or bad.
>>>
>> I didn't take it as such, I'm just not sure what's the best approach here,
>> so I added and looked up some more information
>>
>>> I was only telling you that the portion inside the bcm2835_i2c_xfer:
>>> + /* Combined write-read to the same address (smbus) */
>>> + if (num == 2 && (msgs[0].addr == msgs[1].addr) &&
>>> + !(msgs[0].flags & I2C_M_RD) && (msgs[1].flags & I2C_M_RD) &&
>>> + (msgs[0].len <= 16)) {
>>> + ret = bcm2835_i2c_xfer_msg(i2c_dev, &msgs[0], &msgs[1]);
>>> +
>>> + return ret ? ret : 2;
>>> + }
>>> is very specific and maybe could be done in a "generic" manner
>>> supporting more cases.
>>>
>> It has to be specific when it comes to number of messages. We can only
>> support ReStart after the first message unless we use polling for the
>> whole transfer. And in that case we can't disable interrupts for such
>> a long period and we will end up sometimes loosing Transfer Active,
>> resulting in Stop Condition between the messages.
>> So we can only do transfers with 2 messages if we want Restart.
>>
>> It is possible to support more than 16 bytes for the first message,
>> filling the FIFO after polling TA, but I'm not sure that is common.
>> Mostly it's 1 or 2 bytes to set a register.
>> The write-read restriction isn't absolutely necessary either, but it's the
>> most common case I think. So it was about reusing bcm2835_i2c_xfer_msg().
>> A less restrictive approach would require a dedicated function I think.
>>
>>> At least add a dev_warn_once for all num > 1 cases not handled by the
>>> code above.
>>>
>>> This gives people an opportunity to detect such a situation if they
>>> find something is not working as expected.
>>>
>> I agree.
>>
>> After reading joan's report I wonder if it would be best to add a module
>> parameter like downstream has, so it can be disabled. What do you think?
>>
> I guess let us start simple:
> * get warning in place about always issuing a stop for num > 1
> - instead we may just want to set max_num_msgs = 1 in quirks.
> * apply your patch for the write (<=16) then read case.
> - maybe by setting quirks I2C_AQ_COMB_WRITE_THEN_READ
> plus max_comb_1st_msg_len = 16 and max_num_msgs = 2
>
> If this becomes too restrictive for some specific HW, then someone
> may want to add the missing features.
>
> As for the module parameters: no idea if this is acceptable
> or sensible.
>
> But that’s just my 2c...
It suddenly struct me that I had seen the TA bit set when I debugged the
interrupt function. And it turns out that if I don't prefill the FIFO,
then I can use the TXW interrupt to know when the transfer is active,
no need for polling. It looks promising so far, need to run my testcases
to be sure.
Thanks,
Noralf.
^ permalink raw reply
* Re: [PATCH v5 1/6] I2C: i2c-smbus: add device tree support
From: Mark Rutland @ 2016-09-21 11:34 UTC (permalink / raw)
To: Phil Reid
Cc: wsa, robh+dt, sre, andrea.merello, karl-heinz, arnd, linux-i2c,
devicetree, linux-pm
In-Reply-To: <1474447274-90821-2-git-send-email-preid@electromag.com.au>
On Wed, Sep 21, 2016 at 04:41:09PM +0800, Phil Reid wrote:
> From: Andrea Merello <andrea.merello@gmail.com>
>
> According to Documentation/i2c/smbus-protocol, a smbus controller driver
> that wants to hook-in smbus extensions support, can call
> i2c_setup_smbus_alert(). There are very few drivers that are currently
> doing this.
>
> However the i2c-smbus module can also work with any
> smbus-extensions-unaware I2C controller, as long as we provide an extra
> IRQ line connected to the I2C bus ALARM signal.
>
> This patch makes it possible to go this way via DT. Note that the DT node
> will indeed describe a (little) piece of HW, that is the routing of the
> ALARM signal to an IRQ line (so it seems a fair DT use to me, but RFC).
Which piece of hardware actually generates this IRQ? The I2C controller?
A slave SMBus device? Or something else?
I'm not at all familiar with I2C or SMBus, and a quick scan of
Documentation/i2c/smbus-protocol, has left me none-the-wiser on that
front.
> Note that AFAICT, by design, i2c-smbus module pretends to be an I2C slave
> with address 0x0C (that is the alarm response address), and IMHO this is
> quite consistent with usage in the DT as a I2C child node.
>
> Signed-off-by: Andrea Merello <andrea.merello@gmail.com>
> Signed-off-by: Phil Reid <preid@electromag.com.au>
> ---
> Documentation/devicetree/bindings/i2c/i2c-smbus.txt | 20 ++++++++++++++++++++
> drivers/i2c/i2c-smbus.c | 20 +++++++++++++++-----
> 2 files changed, 35 insertions(+), 5 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/i2c/i2c-smbus.txt
>
> diff --git a/Documentation/devicetree/bindings/i2c/i2c-smbus.txt b/Documentation/devicetree/bindings/i2c/i2c-smbus.txt
> new file mode 100644
> index 0000000..da83127
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/i2c/i2c-smbus.txt
> @@ -0,0 +1,20 @@
> +* i2c-smbus extensions
> +
> +Required Properties:
> + - compatible: Must contain "smbus_alert"
Nit: s/_/-/ in compatible strings please.
> + - interrupts: The irq line for smbus ALERT signal
> + - reg: I2C slave address. Set to 0x0C (alert response address).
> +
> +Note: The i2c-smbus module registers itself as a slave I2C device. Whenever
> +a smbus controller directly support smbus extensions (and its driver supports
> +this), there is no need to add anything special to the DT. Otherwise, for using
> +i2c-smbus with any smbus-extensions-unaware I2C controllers, you need to
> +route the smbus ALARM signal to an extra IRQ line, thus you need to describe
> +this in the DT.
Bindings shouldn't mention driver details (e.g. the i2c-smbus module
behaviour). It feels like we're creating a virtual device for the sake
of a driver, rather than accurately capturing the hardware.
So as-is, I don't think this is the right way to describe this.
Thanks,
Mark.
^ permalink raw reply
* Re: [PATCH v2 0/4] gpio: fix an incorrect lockdep warning
From: Peter Rosin @ 2016-09-21 9:47 UTC (permalink / raw)
To: Thomas Gleixner
Cc: Peter Zijlstra, Bartosz Golaszewski, Wolfram Sang, Linus Walleij,
Alexandre Courbot, Andy Shevchenko, Vignesh R, Yong Li,
Geert Uytterhoeven, Ingo Molnar, linux-i2c, linux-gpio, LKML
In-Reply-To: <alpine.DEB.2.20.1609201725550.6905@nanos>
On 2016-09-20 17:33, Thomas Gleixner wrote:
> On Tue, 20 Sep 2016, Peter Rosin wrote:
>> On 2016-09-19 11:03, Peter Zijlstra wrote:
>>>
>>> Use the -RT kernel and all locks will end up as rt_mutex. Avoiding
>>> inversion on one specific lock, while there are then a gazillion other
>>> than can equally create inversion doesn't make sense to me.
>
> That's true, but the locking of the i2c stuff is pretty much self contained
> and the results of using an rtmutex speak for themself.
>
> One of the issues is that i2c needs to use threaded interrupt handlers and
> blocking out the handler thread with a preempted user space task is hurting
> performance badly.
>
> I don't think that using a rtmutex there is wrong. It cures at least a very
> clear priority inversion issue versus the threaded interrupt handler.
>
> Forcing all i2c users off to RT is not really an option. RT has other
> drawbacks vs. throughput which you don't want to impose on everything which
> happens to use i2c.
Oh, we're not talking about forcing *all* i2c users off to RT, only
those that suffer from the priority inversion. The original report
leading to the bus_lock changing to a rt_mutex seemed like if
someone had suggested an -RT kernel instead of the patch being
accepted, it would have been the better option. If -RT was a viable
option back then? But ok, that was a long time ago and by now
there's no way to be sure what troubles a change back to an ordinary
mutex is going cause. The problem is of course that there is no way
of knowing what, if anything, will suffer from inversion with
ordinary mutexes instead of rt_mutexes.
And this is also true about changing mux_lock to an ordinary mutex.
There is simply no way of knowing if someone depends on avoiding
inversion in a muxing scenario, that dependency may have developed
at any point in time and is not related to the relatively recent
addition of the mux_lock. So, there is a greater risk for
regressions with turning mux_lock into a mutex than I originally
thought.
>> 2a will cause a lockdep splat if i2c0:mux_lock is in the same
>> lockdep class & subclass as i2c1:mux_lock. So, lockdep needs
>> separate lockdep classes depending on the i2c root adapter
>> (subclasses are needed to handle deeper trees, so they are off
>> limits). Great fun. How do I go about creating a new lockdep
>> class for every i2c root adapter instance?
Bzzzt. It is not enough to have one class per root adapter, just
move everything down one mux level:
.---. .----.
| | | |-- i2c3
| | .-|mux1| .----.
| l | .----. / | |-- i2c4 --|gpio|
| i | | |-- i2c1 -' '----' '----'
| n |-- i2c0 --|mux0| .--------------'
| u | | |-- i2c2 -. .----. .----.
| x | '----' \ | |-- i2c5 --|dev0|
| | '-|mux2| '----'
| | | |-- i2c6
'---' '----'
access dev0 on i2c5
1. lock i2c2:mux_lock
2. switch mux2 to i2c5 using gpio on i2c4
a lock i2c1:mux_lock
b switch mux1 to i2c4 using whatever
c access gpio on i2c1->i2c4
i lock i2c0:mux_lock
ii switch mux0 to i2c1 using whatever
iii access gpio on i2c0->i2c1->i2c4
iv unlock i2c0:mux_lock
d unlock i2c1:mux_lock
3. access dev0 on i2c2->i2c5
a lock i2c0:mux_lock
b switch mux0 to i2c2 using whatever
c access dev0 on i2c0->i2c2->i2c5
d unlock i2c0:mux_lock
4. unlock i2c2:mux_lock
2a is the problematic step here too. i2c1:mux_lock and i2c2:mux_lock
are on the same depth, so needs fully separate lockdep classes. Which
means that using the adapter depth to set the lockdep subclass
is pointless. And since they are branches on the same root adapter,
using one lockdep class per root adapter also falls apart.
> lockdep_set_class() .....
Right, thanks. But given the above, the only way I can think of to
reduce the number of lockdep classes is to defer creating a lockdep
class for an adapter until a mux is attached as a child adapter.
But having extra code to reduce lockdep classes for mux_lock is
pointless, since first of all given the above risk for regression,
I'm no longer all that happy about the rt_mutex -> mutex conversion
even for mux_lock. Then the only reason to consider lockdep classes
is if someone adds support for lockdep to the rt_mutex, which is
on the todo anyway. By then I bet the lockdep class problem will also
present itself for the bus_lock in some manner with some form of
interdependent devices requiring one lockdep class per i2c adapter
anyway.
So, bottom line is that the best we can probably do is to create
one lockdep class per instantiated adapter (i.e. 7 classes for
the above tree), and use one subclass for bus_lock and one for
mux_lock. And the problem is of course that a lockdep class will
be leaked on each de-registration of an i2c adapter.
i2c-based gpio expanders could piggy-back on that and use the same
lockdep class as the i2c adapter it sits on, but use a special
"gpio" subclass. I don't know if that scales though? I mean, can
two drivers for different i2c gpio expanders share lockdep subclass
in the general case?
We can possibly defer the creation of the lockdep class until
either a client or a mux is attached to the adapter in order to
reduce the lockdep class leakage to actual in-use adapters,
whatever that may be worth...
Cheers,
Peter
^ permalink raw reply
* Re: [PATCH v2] i2c / ACPI: Do not touch an I2C device if it belongs to another adapter
From: Mika Westerberg @ 2016-09-21 8:45 UTC (permalink / raw)
To: Wolfram Sang
Cc: Nicolai Stange, Octavian Purdila, Rafael J . Wysocki,
Jarkko Nikula, linux-i2c, linux-kernel
In-Reply-To: <20160921054834.GC1484@katana>
On Wed, Sep 21, 2016 at 07:48:35AM +0200, Wolfram Sang wrote:
> On Tue, Sep 20, 2016 at 04:59:25PM +0300, Mika Westerberg wrote:
> > When enumerating I2C devices connected to an I2C adapter we scan the whole
> > namespace (as it is possible to have devices anywhere in that namespace,
> > not just below the I2C adapter device) and add each found device to the I2C
> > bus in question.
> >
> > Now after commit 525e6fabeae2 ("i2c / ACPI: add support for ACPI
> > reconfigure notifications") checking of the adapter handle to the one found
> > in the I2cSerialBus() resource was moved to happen after resources of the
> > I2C device has been parsed. This means that if the I2cSerialBus() resource
> > points to an adapter that does not exists in the system we still parse
> > those resources. This is problematic in particular because
> > acpi_dev_resource_interrupt() tries to configure GSI if the device also has
> > an Interrupt() resource. Failing to do that results errrors like this to be
> > printed on the console:
> >
> > [ 10.409490] ERROR: Unable to locate IOAPIC for GSI 37
> >
> > To fix this we pass the I2C adapter to i2c_acpi_get_info() and make sure
> > the handle matches the one in the I2cSerialBus() resource before doing
> > anything else to the device.
> >
> > Reported-and-tested-by: Nicolai Stange <nicstange@gmail.com>
> > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
>
> Considering this for for-current. So shall we add:
>
> Fixes: 525e6fabeae2 ("i2c / ACPI: add support for ACPI reconfigure notifications")
>
> ?
Yes please :)
^ permalink raw reply
* [PATCH v5 0/6] power: supply: sbs-manager add driver.
From: Phil Reid @ 2016-09-21 8:41 UTC (permalink / raw)
To: wsa, robh+dt, mark.rutland, sre, andrea.merello, preid,
karl-heinz, arnd, linux-i2c, devicetree, linux-pm
The sbs-manager driver was written by Karl-Heinz and I've added irq
support to the driver to generate power_supply_change notices.
Hope I'm not violating etticate too much with this series
I've include Andrea's smb_alert driver changes here primarily to allow
testing of the alert callback. It looks like those changes stalled.
I'd be keen to implement necessary changes to this to get dt support
for the smbus_alert driver into mainline. Just need some guidance on
how.
Patches are applied against linux-power-supply/for-next
This patch series adds support for Smart Battery System Manager.
A SBSM is a device listening at I2C/SMBus address 0x0a and is capable of
communicating up to four I2C smart battery devices. All smart battery
devices are listening at address 0x0b, so the SBSM muliplexes between
them. The driver makes use of the I2C-Mux framework to allow smart
batteries to be bound via device tree, i.e. the sbs-battery driver.
Via sysfs interface the online state and charge type are presented. If
the driver is bound as ltc1760 (a Dual Smart Battery System Manager)
the charge type can also be changed from trickle to fast.
Changes:
V2:
* Fixed sample node names and compatible strings in binding documentaton.
* Removed optional property "sbsm,i2c-retry-count"
* Retry count hardcoded. Removed no longer needed OF readout function
* Rebased onto v4.7rc-5
V3:
* Changed enumeration sheme
* Rebased onto v4.7rc-7
V4:
* removed .owner field as advised by kbuild test robot
* removed print on ENOMEM
* added reference to i2c-mux documentation in binding doc
V5: v2 of my (Phil) Add irq support and battery detect gpios.
* update karl's sbs-manager binding to include gpio defintions.
* rebase onto linux-power-supply/for-next
* Add alert callback to sbs-manager.
* Expose battery presence via gpio to maintain compatiblity with
sbs-battery driver.
* Notify attached batteries of state change via alert callback.
This was done to avoid the need for the driver to become an irq
source. This reduces the code a fair bit.
Hopefully the implementation meets with approval.
Andrea Merello (1):
I2C: i2c-smbus: add device tree support
Karl-Heinz Schneider (2):
Documentation: Add sbs-manager device tree node documentation
power: Adds support for Smart Battery System Manager
Phil Reid (3):
i2c: i2c-smbus: Support threaded irq.
power: supply: sbs-battery: Add alert callback.
power: supply: sbs-manager: Add alert callback and battery change
notification.
.../devicetree/bindings/i2c/i2c-smbus.txt | 20 +
.../bindings/power/supply/sbs,sbs-manager.txt | 62 +++
drivers/i2c/i2c-smbus.c | 41 +-
drivers/power/supply/Kconfig | 13 +
drivers/power/supply/Makefile | 1 +
drivers/power/supply/sbs-battery.c | 16 +-
drivers/power/supply/sbs-manager.c | 451 +++++++++++++++++++++
7 files changed, 591 insertions(+), 13 deletions(-)
create mode 100644 Documentation/devicetree/bindings/i2c/i2c-smbus.txt
create mode 100644 Documentation/devicetree/bindings/power/supply/sbs,sbs-manager.txt
create mode 100644 drivers/power/supply/sbs-manager.c
--
1.8.3.1
^ permalink raw reply
* [PATCH v5 4/6] power: Adds support for Smart Battery System Manager
From: Phil Reid @ 2016-09-21 8:41 UTC (permalink / raw)
To: wsa, robh+dt, mark.rutland, sre, andrea.merello, preid,
karl-heinz, arnd, linux-i2c, devicetree, linux-pm
In-Reply-To: <1474447274-90821-1-git-send-email-preid@electromag.com.au>
From: Karl-Heinz Schneider <karl-heinz@schneider-inet.de>
This patch adds support for Smart Battery System Manager.
A SBSM is a device listening at I2C/SMBus address 0x0a and is capable of
communicating up to four I2C smart battery devices. All smart battery
devices are listening at address 0x0b, so the SBSM muliplexes between
them. The driver makes use of the I2C-Mux framework to allow smart
batteries to be bound via device tree, i.e. the sbs-battery driver.
Via sysfs interface the online state and charge type are presented. If
the driver is bound as ltc1760 (an implementation of a Dual Smart Battery
System Manager) the charge type can also be changed from trickle to fast.
Tested-by: Phil Reid <preid@electromag.com.au>
Reviewed-by: Phil Reid <preid@electromag.com.au>
Signed-off-by: Karl-Heinz Schneider <karl-heinz@schneider-inet.de>
Signed-off-by: Phil Reid <preid@electromag.com.au>
---
drivers/power/supply/Kconfig | 13 ++
drivers/power/supply/Makefile | 1 +
drivers/power/supply/sbs-manager.c | 323 +++++++++++++++++++++++++++++++++++++
3 files changed, 337 insertions(+)
create mode 100644 drivers/power/supply/sbs-manager.c
diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
index 76806a0..2666ec8 100644
--- a/drivers/power/supply/Kconfig
+++ b/drivers/power/supply/Kconfig
@@ -157,6 +157,19 @@ config BATTERY_WM97XX
help
Say Y to enable support for battery measured by WM97xx aux port.
+config MANAGER_SBS
+ tristate "Smart Battery System Manager"
+ depends on I2C && I2C_MUX
+ help
+ Say Y here to include support for Smart Battery System Manager
+ ICs. The driver reports online and charging status via sysfs.
+ It presents itself also as I2C mux which allows to bind
+ smart battery driver to its ports.
+ Supported is for example LTC1760.
+
+ This driver can also be built as a module. If so, the module will be
+ called sbs-manager.
+
config BATTERY_SBS
tristate "SBS Compliant gas gauge"
depends on I2C
diff --git a/drivers/power/supply/Makefile b/drivers/power/supply/Makefile
index 36c599d..77f4914 100644
--- a/drivers/power/supply/Makefile
+++ b/drivers/power/supply/Makefile
@@ -22,6 +22,7 @@ obj-$(CONFIG_BATTERY_DS2760) += ds2760_battery.o
obj-$(CONFIG_BATTERY_DS2780) += ds2780_battery.o
obj-$(CONFIG_BATTERY_DS2781) += ds2781_battery.o
obj-$(CONFIG_BATTERY_DS2782) += ds2782_battery.o
+obj-$(CONFIG_MANAGER_SBS) += sbs-manager.o
obj-$(CONFIG_BATTERY_GAUGE_LTC2941) += ltc2941-battery-gauge.o
obj-$(CONFIG_BATTERY_GOLDFISH) += goldfish_battery.o
obj-$(CONFIG_BATTERY_PMU) += pmu_battery.o
diff --git a/drivers/power/supply/sbs-manager.c b/drivers/power/supply/sbs-manager.c
new file mode 100644
index 0000000..e3debd7
--- /dev/null
+++ b/drivers/power/supply/sbs-manager.c
@@ -0,0 +1,323 @@
+/*
+ * Driver for SBS compliant Smart Battery System Managers
+ *
+ * The device communicates via i2c at address 0x0a and multiplexes access to up
+ * to four smart batteries at address 0x0b.
+ *
+ * Via sysfs interface the online state and charge type are presented.
+ *
+ * Datasheet SBSM: http://sbs-forum.org/specs/sbsm100b.pdf
+ * Datasheet LTC1760: http://cds.linear.com/docs/en/datasheet/1760fb.pdf
+ *
+ * Karl-Heinz Schneider <karl-heinz@schneider-inet.de>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/module.h>
+#include <linux/i2c.h>
+#include <linux/i2c-mux.h>
+#include <linux/power_supply.h>
+
+#define SBSM_MAX_BATS 4
+#define SBSM_RETRY_CNT 3
+
+/* registers addresses */
+#define SBSM_CMD_BATSYSSTATE 0x01
+#define SBSM_CMD_BATSYSSTATECONT 0x02
+#define SBSM_CMD_BATSYSINFO 0x04
+#define SBSM_CMD_LTC 0x3c
+
+struct sbsm_data {
+ struct i2c_client *client;
+ struct i2c_mux_core *muxc;
+
+ struct power_supply *psy;
+
+ int cur_chan; /* currently selected channel */
+ bool is_ltc1760; /* special capabilities */
+};
+
+static enum power_supply_property sbsm_props[] = {
+ POWER_SUPPLY_PROP_ONLINE,
+ POWER_SUPPLY_PROP_CHARGE_TYPE,
+};
+
+static int sbsm_read_word(struct i2c_client *client, u8 address)
+{
+ int reg, retries = SBSM_RETRY_CNT;
+
+ while (retries > 0) {
+ reg = i2c_smbus_read_word_data(client, address);
+ if (reg >= 0)
+ break;
+ --retries;
+ }
+
+ if (reg < 0) {
+ dev_err(&client->dev, "failed to read register %i\n",
+ (int)address);
+ return reg;
+ }
+
+ return le16_to_cpu(reg);
+}
+
+static int sbsm_write_word(struct i2c_client *client, u8 address, u16 word)
+{
+ int ret, retries = SBSM_RETRY_CNT;
+
+ word = cpu_to_le16(word);
+ while (retries > 0) {
+ ret = i2c_smbus_write_word_data(client, address, word);
+ if (ret >= 0)
+ break;
+ --retries;
+ }
+ if (ret < 0)
+ dev_err(&client->dev, "failed to write to register %i\n",
+ (int)address);
+
+ return ret;
+}
+
+static int sbsm_get_property(struct power_supply *psy,
+ enum power_supply_property psp,
+ union power_supply_propval *val)
+{
+ struct sbsm_data *data = power_supply_get_drvdata(psy);
+ int regval = 0;
+
+ switch (psp) {
+ case POWER_SUPPLY_PROP_ONLINE:
+ regval = sbsm_read_word(data->client, SBSM_CMD_BATSYSSTATECONT);
+ if (regval < 0)
+ return regval;
+ val->intval = !!(regval & 0x1);
+ break;
+
+ case POWER_SUPPLY_PROP_CHARGE_TYPE:
+ regval = sbsm_read_word(data->client, SBSM_CMD_BATSYSSTATE);
+ if (regval < 0)
+ return regval;
+
+ if ((regval & 0x00f0) == 0) {
+ val->intval = POWER_SUPPLY_CHARGE_TYPE_NONE;
+ return 0;
+ }
+ val->intval = POWER_SUPPLY_CHARGE_TYPE_TRICKLE;
+
+ if (data->is_ltc1760) {
+ /* charge mode fast if turbo is active */
+ regval = sbsm_read_word(data->client, SBSM_CMD_LTC);
+ if (regval < 0)
+ return regval;
+ else if (regval & 0x80)
+ val->intval = POWER_SUPPLY_CHARGE_TYPE_FAST;
+ }
+ break;
+
+ default:
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static int sbsm_prop_is_writeable(struct power_supply *psy,
+ enum power_supply_property psp)
+{
+ struct sbsm_data *data = power_supply_get_drvdata(psy);
+
+ return (psp == POWER_SUPPLY_PROP_CHARGE_TYPE) && data->is_ltc1760;
+}
+
+static int sbsm_set_property(struct power_supply *psy,
+ enum power_supply_property psp,
+ const union power_supply_propval *val)
+{
+ struct sbsm_data *data = power_supply_get_drvdata(psy);
+ int ret = -EINVAL;
+
+ switch (psp) {
+ case POWER_SUPPLY_PROP_CHARGE_TYPE:
+ /* write 1 to TURBO if type fast is given */
+ if (data->is_ltc1760) {
+ u16 regval = val->intval ==
+ POWER_SUPPLY_CHARGE_TYPE_FAST ? (0x1 << 7) : 0;
+ ret = sbsm_write_word(data->client, SBSM_CMD_LTC,
+ regval);
+ }
+ break;
+
+ default:
+ break;
+ }
+
+ return ret;
+}
+
+/*
+ * Switch to battery
+ * Parameter chan is directly the content of SMB_BAT* nibble
+ */
+static int sbsm_select(struct i2c_mux_core *muxc, u32 chan)
+{
+ struct sbsm_data *data = i2c_mux_priv(muxc);
+ int ret = 0;
+ u16 reg;
+
+ if (data->cur_chan == chan)
+ return ret;
+
+ /* chan goes from 1 ... 4 */
+ reg = 1 << (11 + chan);
+ ret = sbsm_write_word(data->client, SBSM_CMD_BATSYSSTATE, reg);
+ if (ret)
+ dev_err(&data->client->dev, "Failed to select channel %i\n",
+ chan);
+ else
+ data->cur_chan = chan;
+
+ return ret;
+}
+
+#if defined(CONFIG_OF)
+
+#include <linux/of_device.h>
+
+static const struct of_device_id sbsm_dt_ids[] = {
+ { .compatible = "sbs,sbs-manager" },
+ { .compatible = "lltc,ltc1760" },
+ { }
+};
+MODULE_DEVICE_TABLE(of, sbsm_dt_ids);
+
+#endif
+
+static const struct power_supply_desc sbsm_default_psy_desc = {
+ .type = POWER_SUPPLY_TYPE_MAINS,
+ .properties = sbsm_props,
+ .num_properties = ARRAY_SIZE(sbsm_props),
+ .get_property = &sbsm_get_property,
+ .set_property = &sbsm_set_property,
+ .property_is_writeable = &sbsm_prop_is_writeable,
+};
+
+static int sbsm_probe(struct i2c_client *client,
+ const struct i2c_device_id *id)
+{
+ struct i2c_adapter *adapter = to_i2c_adapter(client->dev.parent);
+ struct sbsm_data *data;
+ struct device *dev = &client->dev;
+ struct power_supply_desc *psy_desc;
+ struct power_supply_config psy_cfg = {};
+ int ret = 0, i, supported_bats;
+
+ /* Device listens only at address 0x0a */
+ if (client->addr != 0x0a)
+ return -ENODEV;
+
+ if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_WORD_DATA))
+ return -EPFNOSUPPORT;
+
+ data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
+ if (!data)
+ return -ENOMEM;
+
+ i2c_set_clientdata(client, data);
+
+ data->client = client;
+ data->is_ltc1760 = !!strstr(id->name, "ltc1760");
+
+ ret = sbsm_read_word(client, SBSM_CMD_BATSYSINFO);
+ if (ret < 0)
+ return ret;
+ supported_bats = le16_to_cpu(ret) & 0xf;
+
+ data->muxc = i2c_mux_alloc(adapter, dev, SBSM_MAX_BATS, 0,
+ I2C_MUX_LOCKED, &sbsm_select, NULL);
+ if (!data->muxc) {
+ ret = -ENOMEM;
+ goto err_mux_alloc;
+ }
+ data->muxc->priv = data;
+
+ /* register muxed i2c channels. One for each supported battery */
+ for (i = 0; i < SBSM_MAX_BATS; ++i) {
+ if ((1 << i) & supported_bats) {
+ ret = i2c_mux_add_adapter(data->muxc, 0, i + 1, 0);
+ if (ret) {
+ dev_err(dev,
+ "failed to register i2c mux channel %d\n",
+ i + 1);
+ goto err_mux_register;
+ }
+ }
+ }
+
+ psy_desc = devm_kmemdup(dev, &sbsm_default_psy_desc,
+ sizeof(struct power_supply_desc),
+ GFP_KERNEL);
+ if (!psy_desc) {
+ ret = -ENOMEM;
+ goto err_psy;
+ }
+
+ psy_desc->name = devm_kasprintf(dev, GFP_KERNEL, "sbsm-%s",
+ dev_name(&client->dev));
+ if (!psy_desc->name) {
+ ret = -ENOMEM;
+ goto err_psy;
+ }
+
+ psy_cfg.drv_data = data;
+ data->psy = devm_power_supply_register(dev, psy_desc, &psy_cfg);
+ if (IS_ERR(data->psy)) {
+ ret = PTR_ERR(data->psy);
+ dev_err(dev, "failed to register power supply %s\n",
+ psy_desc->name);
+ goto err_psy;
+ }
+
+ dev_info(dev, "sbsm registered\n");
+ return 0;
+
+err_psy:
+err_mux_register:
+ i2c_mux_del_adapters(data->muxc);
+
+err_mux_alloc:
+ return ret;
+}
+
+static int sbsm_remove(struct i2c_client *client)
+{
+ struct sbsm_data *data = i2c_get_clientdata(client);
+
+ i2c_mux_del_adapters(data->muxc);
+ return 0;
+}
+
+static const struct i2c_device_id sbsm_ids[] = {
+ { "sbs-manager", 0 },
+ { "ltc1760", 0 },
+ { }
+};
+MODULE_DEVICE_TABLE(i2c, sbsm_ids);
+
+static struct i2c_driver sbsm_driver = {
+ .driver = {
+ .name = "sbsm",
+ },
+ .probe = sbsm_probe,
+ .remove = sbsm_remove,
+ .id_table = sbsm_ids
+};
+module_i2c_driver(sbsm_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Karl-Heinz Schneider <karl-heinz@schneider-inet.de>");
+MODULE_DESCRIPTION("SBSM Smart Battery System Manager");
--
1.8.3.1
^ permalink raw reply related
* [PATCH v5 6/6] power: supply: sbs-manager: Add alert callback and battery change notification.
From: Phil Reid @ 2016-09-21 8:41 UTC (permalink / raw)
To: wsa, robh+dt, mark.rutland, sre, andrea.merello, preid,
karl-heinz, arnd, linux-i2c, devicetree, linux-pm
In-Reply-To: <1474447274-90821-1-git-send-email-preid@electromag.com.au>
This adds irq support via the smbus_alert driver to generate
power_supply_changed notifications when either external power is
removed / applied or a battery inserted / removed.
Uuse the i2c alert callback to notify the attached battery driver that a
change has occurred.
Signed-off-by: Phil Reid <preid@electromag.com.au>
---
drivers/power/supply/sbs-manager.c | 134 ++++++++++++++++++++++++++++++++++++-
1 file changed, 131 insertions(+), 3 deletions(-)
diff --git a/drivers/power/supply/sbs-manager.c b/drivers/power/supply/sbs-manager.c
index e3debd7..9b92b40 100644
--- a/drivers/power/supply/sbs-manager.c
+++ b/drivers/power/supply/sbs-manager.c
@@ -19,6 +19,7 @@
#include <linux/module.h>
#include <linux/i2c.h>
#include <linux/i2c-mux.h>
+#include <linux/gpio.h>
#include <linux/power_supply.h>
#define SBSM_MAX_BATS 4
@@ -30,14 +31,22 @@
#define SBSM_CMD_BATSYSINFO 0x04
#define SBSM_CMD_LTC 0x3c
+#define SBSM_BIT_AC_PRESENT BIT(0)
+
struct sbsm_data {
struct i2c_client *client;
struct i2c_mux_core *muxc;
struct power_supply *psy;
+ struct gpio_chip chip;
+
int cur_chan; /* currently selected channel */
bool is_ltc1760; /* special capabilities */
+
+ unsigned int supported_bats;
+ unsigned int last_state;
+ unsigned int last_state_cont;
};
static enum power_supply_property sbsm_props[] = {
@@ -184,6 +193,121 @@ static int sbsm_select(struct i2c_mux_core *muxc, u32 chan)
return ret;
}
+static int sbsm_gpio_get_value(struct gpio_chip *gc, unsigned off)
+{
+ struct sbsm_data *data = gpiochip_get_data(gc);
+ int ret;
+
+ ret = sbsm_read_word(data->client, SBSM_CMD_BATSYSSTATE);
+ if (ret < 0)
+ return ret;
+
+ return ret & BIT(off);
+}
+
+/*
+* This needs to be defined or the GPIO lib fails to register the pin.
+* But the 'gpio' is always an input.
+*/
+static int sbsm_gpio_direction_input(struct gpio_chip *gc, unsigned off)
+{
+ return 0;
+}
+
+static int sbsm_do_alert(struct device *dev, void *d)
+{
+ struct i2c_client *client = i2c_verify_client(dev);
+ struct i2c_driver *driver;
+
+ if (!client || client->addr != 0x0b)
+ return 0;
+
+ /*
+ * Drivers should either disable alerts, or provide at least
+ * a minimal handler. Lock so the driver won't change.
+ */
+ device_lock(dev);
+ if (client->dev.driver) {
+ driver = to_i2c_driver(client->dev.driver);
+ if (driver->alert)
+ driver->alert(client, I2C_PROTOCOL_SMBUS_ALERT, 0);
+ else
+ dev_warn(&client->dev, "no driver alert()!\n");
+ } else
+ dev_dbg(&client->dev, "alert with no driver\n");
+ device_unlock(dev);
+
+ /* Stop iterating after we find the device */
+ return -EBUSY;
+}
+
+void sbsm_alert(struct i2c_client *client, enum i2c_alert_protocol prot,
+ unsigned int d)
+{
+ struct sbsm_data *sbsm = i2c_get_clientdata(client);
+
+ int ret, i, irq_bat = 0;
+
+ ret = sbsm_read_word(sbsm->client, SBSM_CMD_BATSYSSTATE);
+ if (ret >= 0)
+ irq_bat = ret ^ sbsm->last_state;
+ sbsm->last_state = ret;
+
+ ret = sbsm_read_word(sbsm->client, SBSM_CMD_BATSYSSTATECONT);
+ if ((ret >= 0) &&
+ ((ret ^ sbsm->last_state_cont) & SBSM_BIT_AC_PRESENT)) {
+ irq_bat |= sbsm->supported_bats;
+ power_supply_changed(sbsm->psy);
+ }
+ sbsm->last_state_cont = ret;
+
+ for (i = 0; i < SBSM_MAX_BATS; i++) {
+ if (irq_bat & BIT(i)) {
+ device_for_each_child(&sbsm->muxc->adapter[i]->dev, 0,
+ sbsm_do_alert);
+ }
+ }
+}
+
+static int sbsm_gpio_setup(struct sbsm_data *data)
+{
+ struct gpio_chip *gc = &data->chip;
+ struct i2c_client *client = data->client;
+ struct device *dev = &client->dev;
+ struct device_node *of_node = client->dev.of_node;
+ int ret;
+
+ if (!of_get_property(of_node, "gpio-controller", NULL))
+ return 0;
+
+ ret = sbsm_read_word(client, SBSM_CMD_BATSYSSTATE);
+ if (ret < 0)
+ return ret;
+ data->last_state = ret;
+
+ ret = sbsm_read_word(client, SBSM_CMD_BATSYSSTATECONT);
+ if (ret < 0)
+ return ret;
+ data->last_state_cont = ret;
+
+ gc->get = sbsm_gpio_get_value;
+ gc->direction_input = sbsm_gpio_direction_input;
+ gc->can_sleep = true;
+ gc->base = -1;
+ gc->ngpio = SBSM_MAX_BATS;
+ gc->label = client->name;
+ gc->parent = dev;
+ gc->owner = THIS_MODULE;
+
+ ret = devm_gpiochip_add_data(dev, gc, data);
+ if (ret) {
+ dev_err(dev, "devm_gpiochip_add_data failed: %d\n", ret);
+ return ret;
+ }
+
+ return ret;
+}
+
#if defined(CONFIG_OF)
#include <linux/of_device.h>
@@ -214,7 +338,7 @@ static int sbsm_probe(struct i2c_client *client,
struct device *dev = &client->dev;
struct power_supply_desc *psy_desc;
struct power_supply_config psy_cfg = {};
- int ret = 0, i, supported_bats;
+ int ret = 0, i;
/* Device listens only at address 0x0a */
if (client->addr != 0x0a)
@@ -235,7 +359,7 @@ static int sbsm_probe(struct i2c_client *client,
ret = sbsm_read_word(client, SBSM_CMD_BATSYSINFO);
if (ret < 0)
return ret;
- supported_bats = le16_to_cpu(ret) & 0xf;
+ data->supported_bats = le16_to_cpu(ret) & 0xf;
data->muxc = i2c_mux_alloc(adapter, dev, SBSM_MAX_BATS, 0,
I2C_MUX_LOCKED, &sbsm_select, NULL);
@@ -247,7 +371,7 @@ static int sbsm_probe(struct i2c_client *client,
/* register muxed i2c channels. One for each supported battery */
for (i = 0; i < SBSM_MAX_BATS; ++i) {
- if ((1 << i) & supported_bats) {
+ if ((1 << i) & data->supported_bats) {
ret = i2c_mux_add_adapter(data->muxc, 0, i + 1, 0);
if (ret) {
dev_err(dev,
@@ -272,6 +396,9 @@ static int sbsm_probe(struct i2c_client *client,
ret = -ENOMEM;
goto err_psy;
}
+ ret = sbsm_gpio_setup(data);
+ if (ret < 0)
+ goto err_psy;
psy_cfg.drv_data = data;
data->psy = devm_power_supply_register(dev, psy_desc, &psy_cfg);
@@ -314,6 +441,7 @@ static struct i2c_driver sbsm_driver = {
},
.probe = sbsm_probe,
.remove = sbsm_remove,
+ .alert = sbsm_alert,
.id_table = sbsm_ids
};
module_i2c_driver(sbsm_driver);
--
1.8.3.1
^ permalink raw reply related
* [PATCH v5 5/6] power: supply: sbs-battery: Add alert callback.
From: Phil Reid @ 2016-09-21 8:41 UTC (permalink / raw)
To: wsa, robh+dt, mark.rutland, sre, andrea.merello, preid,
karl-heinz, arnd, linux-i2c, devicetree, linux-pm
In-Reply-To: <1474447274-90821-1-git-send-email-preid@electromag.com.au>
To simplify the sbs-manager code and notification of battery removal
use the i2c alert callback to notify the sbs-battery driver that an
event has occurred.
Signed-off-by: Phil Reid <preid@electromag.com.au>
---
drivers/power/supply/sbs-battery.c | 16 +++++++++++++---
1 file changed, 13 insertions(+), 3 deletions(-)
diff --git a/drivers/power/supply/sbs-battery.c b/drivers/power/supply/sbs-battery.c
index d15ce9e..b1284e6 100644
--- a/drivers/power/supply/sbs-battery.c
+++ b/drivers/power/supply/sbs-battery.c
@@ -674,21 +674,30 @@ done:
return 0;
}
-static irqreturn_t sbs_irq(int irq, void *devid)
+static void sbs_supply_changed(struct sbs_info *chip)
{
- struct sbs_info *chip = devid;
struct power_supply *battery = chip->power_supply;
int ret;
ret = gpiod_get_value_cansleep(chip->gpio_detect);
if (ret < 0)
- return ret;
+ return;
chip->is_present = ret;
power_supply_changed(battery);
+}
+static irqreturn_t sbs_irq(int irq, void *devid)
+{
+ sbs_supply_changed(devid);
return IRQ_HANDLED;
}
+void sbs_alert(struct i2c_client *client, enum i2c_alert_protocol prot,
+ unsigned int data)
+{
+ sbs_supply_changed(i2c_get_clientdata(client));
+}
+
static void sbs_external_power_changed(struct power_supply *psy)
{
struct sbs_info *chip = power_supply_get_drvdata(psy);
@@ -916,6 +925,7 @@ MODULE_DEVICE_TABLE(of, sbs_dt_ids);
static struct i2c_driver sbs_battery_driver = {
.probe = sbs_probe,
.remove = sbs_remove,
+ .alert = sbs_alert,
.id_table = sbs_id,
.driver = {
.name = "sbs-battery",
--
1.8.3.1
^ permalink raw reply related
* [PATCH v5 3/6] Documentation: Add sbs-manager device tree node documentation
From: Phil Reid @ 2016-09-21 8:41 UTC (permalink / raw)
To: wsa, robh+dt, mark.rutland, sre, andrea.merello, preid,
karl-heinz, arnd, linux-i2c, devicetree, linux-pm
In-Reply-To: <1474447274-90821-1-git-send-email-preid@electromag.com.au>
From: Karl-Heinz Schneider <karl-heinz@schneider-inet.de>
This patch adds device tree documentation for the sbs-manager
Reviewed-by: Phil Reid <preid@electromag.com.au>
Signed-off-by: Karl-Heinz Schneider <karl-heinz@schneider-inet.de>
Signed-off-by: Phil Reid <preid@electromag.com.au>
---
.../bindings/power/supply/sbs,sbs-manager.txt | 62 ++++++++++++++++++++++
1 file changed, 62 insertions(+)
create mode 100644 Documentation/devicetree/bindings/power/supply/sbs,sbs-manager.txt
diff --git a/Documentation/devicetree/bindings/power/supply/sbs,sbs-manager.txt b/Documentation/devicetree/bindings/power/supply/sbs,sbs-manager.txt
new file mode 100644
index 0000000..2a04b30
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/supply/sbs,sbs-manager.txt
@@ -0,0 +1,62 @@
+Binding for sbs-manager
+
+Required properties:
+- compatible: should be "lltc,ltc1760" or use "sbs,sbs-manager" as fallback.
+- reg: integer, i2c address of the device. Should be <0xa>.
+Optional properties:
+- gpio-controller: Marks the port as GPIO controller.
+ See "gpio-specifier" in .../devicetree/bindings/gpio/gpio.txt.
+- #gpio-cells: Should be <2>. The first cell is the pin number, the second cell
+ is used to specify optional parameters:
+ See "gpio-specifier" in .../devicetree/bindings/gpio/gpio.txt.
+
+From OS view the device is basically an i2c-mux used to communicate with up to
+four smart battery devices at address 0xb. The driver actually implements this
+behaviour. So standard i2c-mux nodes can be used to register up to four slave
+batteries. See Documentation/devicetree/bindings/i2c/i2c-mux.txt for more
+information on i2c-mux nodes. Channels will be numerated starting from 1 to 4.
+
+Example:
+
+batman@0a {
+ compatible = "lltc,ltc1760";
+ reg = <0x0a>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ gpio-controller;
+ #gpio-cells = <2>;
+
+ i2c@1 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <1>;
+
+ battery@b {
+ compatible = "ti,bq2060", "sbs,sbs-battery";
+ reg = <0xb>;
+ };
+ };
+
+ i2c@2 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <2>;
+
+ battery@b {
+ compatible = "ti,bq2060", "sbs,sbs-battery";
+ reg = <0xb>;
+ };
+ };
+
+ i2c@3 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <3>;
+
+ battery@b {
+ compatible = "ti,bq2060", "sbs,sbs-battery";
+ reg = <0xb>;
+ };
+ };
+};
--
1.8.3.1
^ permalink raw reply related
* [PATCH v5 2/6] i2c: i2c-smbus: Support threaded irq.
From: Phil Reid @ 2016-09-21 8:41 UTC (permalink / raw)
To: wsa-z923LK4zBo2bacvFa/9K2g, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
mark.rutland-5wv7dgnIgG8, sre-DgEjT+Ai2ygdnm+yROfE0A,
andrea.merello-Re5JQEeQqe8AvxtiuMwx3w,
preid-qgqNFa1JUf/o2iN0hyhwsIdd74u8MsAO,
karl-heinz-X5L7DgJ4l23oE99TX8zNy7NAH6kLmebB, arnd-r2nGTMty4D4,
linux-i2c-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-pm-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1474447274-90821-1-git-send-email-preid-qgqNFa1JUf/o2iN0hyhwsIdd74u8MsAO@public.gmane.org>
handle_nested_irq calls the threaded irq handler. So if the smbus_alert
irq is being generated via this an null address is derefenced. Split
IRQ up into separate functions to allow thread / non thread irq to work
correctly.
Signed-off-by: Phil Reid <preid-qgqNFa1JUf/o2iN0hyhwsIdd74u8MsAO@public.gmane.org>
---
drivers/i2c/i2c-smbus.c | 23 +++++++++++++++++------
1 file changed, 17 insertions(+), 6 deletions(-)
diff --git a/drivers/i2c/i2c-smbus.c b/drivers/i2c/i2c-smbus.c
index 5806db3..b641cc2 100644
--- a/drivers/i2c/i2c-smbus.c
+++ b/drivers/i2c/i2c-smbus.c
@@ -73,13 +73,12 @@ static int smbus_do_alert(struct device *dev, void *addrp)
* The alert IRQ handler needs to hand work off to a task which can issue
* SMBus calls, because those sleeping calls can't be made in IRQ context.
*/
-static void smbus_alert(struct work_struct *work)
+static irqreturn_t smbus_alert(int irq, void *d)
{
- struct i2c_smbus_alert *alert;
+ struct i2c_smbus_alert *alert = d;
struct i2c_client *ara;
unsigned short prev_addr = 0; /* Not a valid address */
- alert = container_of(work, struct i2c_smbus_alert, alert);
ara = alert->ara;
for (;;) {
@@ -116,6 +115,17 @@ static void smbus_alert(struct work_struct *work)
prev_addr = data.addr;
}
+ return IRQ_HANDLED;
+}
+
+static void smbalert_work(struct work_struct *work)
+{
+ struct i2c_smbus_alert *alert;
+
+ alert = container_of(work, struct i2c_smbus_alert, alert);
+
+ smbus_alert(alert->irq, alert);
+
/* We handled all alerts; re-enable level-triggered IRQs */
if (!alert->alert_edge_triggered)
enable_irq(alert->irq);
@@ -158,12 +168,13 @@ static int smbalert_probe(struct i2c_client *ara,
alert->alert_edge_triggered = (irq_type & IRQ_TYPE_EDGE_BOTH);
}
- INIT_WORK(&alert->alert, smbus_alert);
+ INIT_WORK(&alert->alert, smbalert_work);
alert->ara = ara;
if (alert->irq > 0) {
- res = devm_request_irq(&ara->dev, alert->irq, smbalert_irq,
- 0, "smbus_alert", alert);
+ res = devm_request_threaded_irq(&ara->dev, alert->irq,
+ smbalert_irq, smbus_alert,
+ 0, "smbus_alert", alert);
if (res)
return res;
}
--
1.8.3.1
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH v5 1/6] I2C: i2c-smbus: add device tree support
From: Phil Reid @ 2016-09-21 8:41 UTC (permalink / raw)
To: wsa-z923LK4zBo2bacvFa/9K2g, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
mark.rutland-5wv7dgnIgG8, sre-DgEjT+Ai2ygdnm+yROfE0A,
andrea.merello-Re5JQEeQqe8AvxtiuMwx3w,
preid-qgqNFa1JUf/o2iN0hyhwsIdd74u8MsAO,
karl-heinz-X5L7DgJ4l23oE99TX8zNy7NAH6kLmebB, arnd-r2nGTMty4D4,
linux-i2c-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-pm-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1474447274-90821-1-git-send-email-preid-qgqNFa1JUf/o2iN0hyhwsIdd74u8MsAO@public.gmane.org>
From: Andrea Merello <andrea.merello-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
According to Documentation/i2c/smbus-protocol, a smbus controller driver
that wants to hook-in smbus extensions support, can call
i2c_setup_smbus_alert(). There are very few drivers that are currently
doing this.
However the i2c-smbus module can also work with any
smbus-extensions-unaware I2C controller, as long as we provide an extra
IRQ line connected to the I2C bus ALARM signal.
This patch makes it possible to go this way via DT. Note that the DT node
will indeed describe a (little) piece of HW, that is the routing of the
ALARM signal to an IRQ line (so it seems a fair DT use to me, but RFC).
Note that AFAICT, by design, i2c-smbus module pretends to be an I2C slave
with address 0x0C (that is the alarm response address), and IMHO this is
quite consistent with usage in the DT as a I2C child node.
Signed-off-by: Andrea Merello <andrea.merello-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Signed-off-by: Phil Reid <preid-qgqNFa1JUf/o2iN0hyhwsIdd74u8MsAO@public.gmane.org>
---
Documentation/devicetree/bindings/i2c/i2c-smbus.txt | 20 ++++++++++++++++++++
drivers/i2c/i2c-smbus.c | 20 +++++++++++++++-----
2 files changed, 35 insertions(+), 5 deletions(-)
create mode 100644 Documentation/devicetree/bindings/i2c/i2c-smbus.txt
diff --git a/Documentation/devicetree/bindings/i2c/i2c-smbus.txt b/Documentation/devicetree/bindings/i2c/i2c-smbus.txt
new file mode 100644
index 0000000..da83127
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/i2c-smbus.txt
@@ -0,0 +1,20 @@
+* i2c-smbus extensions
+
+Required Properties:
+ - compatible: Must contain "smbus_alert"
+ - interrupts: The irq line for smbus ALERT signal
+ - reg: I2C slave address. Set to 0x0C (alert response address).
+
+Note: The i2c-smbus module registers itself as a slave I2C device. Whenever
+a smbus controller directly support smbus extensions (and its driver supports
+this), there is no need to add anything special to the DT. Otherwise, for using
+i2c-smbus with any smbus-extensions-unaware I2C controllers, you need to
+route the smbus ALARM signal to an extra IRQ line, thus you need to describe
+this in the DT.
+
+Example:
+ alert@0x0C {
+ reg = <0x0C>;
+ compatible = "smbus_alert";
+ interrupts = <0 36 IRQ_TYPE_LEVEL_LOW>;
+ };
diff --git a/drivers/i2c/i2c-smbus.c b/drivers/i2c/i2c-smbus.c
index b0d2679..5806db3 100644
--- a/drivers/i2c/i2c-smbus.c
+++ b/drivers/i2c/i2c-smbus.c
@@ -23,6 +23,7 @@
#include <linux/module.h>
#include <linux/slab.h>
#include <linux/workqueue.h>
+#include <linux/of_irq.h>
struct i2c_smbus_alert {
unsigned int alert_edge_triggered:1;
@@ -139,20 +140,29 @@ static int smbalert_probe(struct i2c_client *ara,
struct i2c_smbus_alert_setup *setup = dev_get_platdata(&ara->dev);
struct i2c_smbus_alert *alert;
struct i2c_adapter *adapter = ara->adapter;
+ struct device_node *of_node = ara->dev.of_node;
int res;
+ int irq_type;
alert = devm_kzalloc(&ara->dev, sizeof(struct i2c_smbus_alert),
GFP_KERNEL);
if (!alert)
return -ENOMEM;
- alert->alert_edge_triggered = setup->alert_edge_triggered;
- alert->irq = setup->irq;
+ if (setup) {
+ alert->alert_edge_triggered = setup->alert_edge_triggered;
+ alert->irq = setup->irq;
+ } else if (of_node) {
+ alert->irq = irq_of_parse_and_map(of_node, 0);
+ irq_type = irq_get_trigger_type(alert->irq);
+ alert->alert_edge_triggered = (irq_type & IRQ_TYPE_EDGE_BOTH);
+ }
+
INIT_WORK(&alert->alert, smbus_alert);
alert->ara = ara;
- if (setup->irq > 0) {
- res = devm_request_irq(&ara->dev, setup->irq, smbalert_irq,
+ if (alert->irq > 0) {
+ res = devm_request_irq(&ara->dev, alert->irq, smbalert_irq,
0, "smbus_alert", alert);
if (res)
return res;
@@ -160,7 +170,7 @@ static int smbalert_probe(struct i2c_client *ara,
i2c_set_clientdata(ara, alert);
dev_info(&adapter->dev, "supports SMBALERT#, %s trigger\n",
- setup->alert_edge_triggered ? "edge" : "level");
+ alert->alert_edge_triggered ? "edge" : "level");
return 0;
}
--
1.8.3.1
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* Re: [v12, 7/8] base: soc: introduce soc_device_match() interface
From: Peter Rosin @ 2016-09-21 8:25 UTC (permalink / raw)
To: Alexander Shiyan, Yangbo Lu
Cc: Mark Rutland, ulf.hansson, Xiaobo Xie, Minghuan Lian, linux-i2c,
linux-clk, Qiang Zhao, Russell King, Bhupesh Sharma, Joerg Roedel,
Jochen Friedrich, Claudiu Manoil, devicetree, Arnd Bergmann,
Scott Wood, Rob Herring, Santosh Shilimkar, linux-arm-kernel,
netdev, linux-mmc, linux-kernel, Leo Li, iommu, Kumar Gala,
linuxppc-dev
In-Reply-To: <1474444574.270880035@f136.i.mail.ru>
On 2016-09-21 09:56, Alexander Shiyan wrote:
>> Среда, 21 сентября 2016, 9:57 +03:00 от Yangbo Lu <yangbo.lu@nxp.com>:
>>
>> From: Arnd Bergmann < arnd@arndb.de >
>>
>> We keep running into cases where device drivers want to know the exact
>> version of the a SoC they are currently running on. In the past, this has
>> usually been done through a vendor specific API that can be called by a
>> driver, or by directly accessing some kind of version register that is
>> not part of the device itself but that belongs to a global register area
>> of the chip.
> ...
>> +const struct soc_device_attribute *soc_device_match(
>> +const struct soc_device_attribute *matches)
>> +{
>> +int ret = 0;
>> +
>> +if (!matches)
>> +return NULL;
>> +
>> +while (!ret) {
>> +if (!(matches->machine || matches->family ||
>> + matches->revision || matches->soc_id))
>> +break;
>> +ret = bus_for_each_dev(&soc_bus_type, NULL, (void *)matches,
>> + soc_device_match_one);
>> +if (!ret)
>> +matches++;
>
> So, what happen if next "matches" (after increment) will be NULL?
A crash?
> I think you should use while(matches) at the start of this procedure.
*arrgh*
*If* matches wrap, you indeed have *big* problems. *Elsewhere*
Hint: Please read the review comments on the previous version of this
series [1] before commenting further.
Cheers,
Peter
[1] https://www.mail-archive.com/netdev@vger.kernel.org/msg126617.html
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [v12, 7/8] base: soc: introduce soc_device_match() interface
From: Alexander Shiyan @ 2016-09-21 7:56 UTC (permalink / raw)
To: Yangbo Lu
Cc: Mark Rutland, ulf.hansson, Xiaobo Xie, Minghuan Lian, linux-i2c,
linux-clk, Qiang Zhao, Russell King, Bhupesh Sharma, Joerg Roedel,
Jochen Friedrich, Claudiu Manoil, devicetree, Arnd Bergmann,
Scott Wood, Rob Herring, Santosh Shilimkar, linux-arm-kernel,
netdev
In-Reply-To: <1474441040-11946-8-git-send-email-yangbo.lu@nxp.com>
>Среда, 21 сентября 2016, 9:57 +03:00 от Yangbo Lu <yangbo.lu@nxp.com>:
>
>From: Arnd Bergmann < arnd@arndb.de >
>
>We keep running into cases where device drivers want to know the exact
>version of the a SoC they are currently running on. In the past, this has
>usually been done through a vendor specific API that can be called by a
>driver, or by directly accessing some kind of version register that is
>not part of the device itself but that belongs to a global register area
>of the chip.
...
>+const struct soc_device_attribute *soc_device_match(
>+const struct soc_device_attribute *matches)
>+{
>+int ret = 0;
>+
>+if (!matches)
>+return NULL;
>+
>+while (!ret) {
>+if (!(matches->machine || matches->family ||
>+ matches->revision || matches->soc_id))
>+break;
>+ret = bus_for_each_dev(&soc_bus_type, NULL, (void *)matches,
>+ soc_device_match_one);
>+if (!ret)
>+matches++;
So, what happen if next "matches" (after increment) will be NULL?
I think you should use while(matches) at the start of this procedure.
---
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox