* Re: [PATCH v4 14/14] i2c: mux: demux-pinctrl: make drivers with no pinctrl work again
From: Geert Uytterhoeven @ 2016-11-07 13:10 UTC (permalink / raw)
To: Wolfram Sang; +Cc: Linux-Renesas, Simon Horman, Linux I2C
In-Reply-To: <20161106202032.5227-15-wsa+renesas@sang-engineering.com>
On Sun, Nov 6, 2016 at 9:20 PM, Wolfram Sang
<wsa+renesas@sang-engineering.com> wrote:
> Some drivers like i2c-gpio do not have dedicated pinctrl states. They
> broke when error checking for pinctrl was added. Detect them now, and in
> their case, simply skip over pinctrl configuration.
>
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> ---
> drivers/i2c/muxes/i2c-demux-pinctrl.c | 22 ++++++++++++++++++++--
> 1 file changed, 20 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/i2c/muxes/i2c-demux-pinctrl.c b/drivers/i2c/muxes/i2c-demux-pinctrl.c
> index b3893f6282ba5b..3e6fe1760d82fc 100644
> --- a/drivers/i2c/muxes/i2c-demux-pinctrl.c
> +++ b/drivers/i2c/muxes/i2c-demux-pinctrl.c
> @@ -69,10 +69,28 @@ static int i2c_demux_activate_master(struct i2c_demux_pinctrl_priv *priv, u32 ne
> goto err_with_revert;
> }
>
> - p = devm_pinctrl_get_select(adap->dev.parent, priv->bus_name);
> + /*
> + * Check if there are pinctrl states at all. Note: we cant' use
can't
> + * devm_pinctrl_get_select() because we need to distinguish between
> + * the -ENODEV from devm_pinctrl_get() and pinctrl_lookup_state().
> + */
> + p = devm_pinctrl_get(adap->dev.parent);
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH v4 01/14] ARM: dts: lager: rename and reindex i2cexio
From: Geert Uytterhoeven @ 2016-11-07 13:12 UTC (permalink / raw)
To: Wolfram Sang; +Cc: Linux-Renesas, Simon Horman, Linux I2C, Simon Horman
In-Reply-To: <20161106202032.5227-2-wsa+renesas@sang-engineering.com>
On Sun, Nov 6, 2016 at 9:20 PM, Wolfram Sang
<wsa+renesas@sang-engineering.com> wrote:
> From: Simon Horman <horms+renesas@verge.net.au>
>
> The rename from i2cexio to i2cexio0 is in in preparation for adding
double "in"
> i2cexio1 which will use the dmuxer for IIC1/I2C1.
demuxer
> The reindexing from i2c8 to i2c10 is to allow space for grouping of
> additional GPIO buses to added by follow-up patches to support demuxing of
to be added
> other i2c buses.
>
> Also note that fallback to GPIO is not provided by the hardware for IIC0/I2C0.
>
> Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
> [wsa: rebased, fixed alias and removed typo in commit message]
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH v3] i2c: designware: Implement support for SMBus block read and write
From: Andy Shevchenko @ 2016-11-07 14:11 UTC (permalink / raw)
To: tnhuynh, Jarkko Nikula, Mika Westerberg, Wolfram Sang, linux-i2c,
linux-kernel
Cc: Loc Ho, Thang Nguyen, Phong Vo, patches
In-Reply-To: <1477896677-13085-1-git-send-email-tnhuynh@apm.com>
On Mon, 2016-10-31 at 13:51 +0700, tnhuynh@apm.com wrote:
> From: Tin Huynh <tnhuynh@apm.com>
>
> Free and Open IPMI use SMBUS BLOCK Read/Write to support SSIF
> protocol.
> However, I2C Designware Core Driver doesn't handle the case at the
> moment.
> The below patch supports this feature.
My comments below.
> @@ -543,6 +543,7 @@ static void i2c_dw_xfer_init(struct dw_i2c_dev
> *dev)
> intr_mask = DW_IC_INTR_DEFAULT_MASK;
>
> for (; dev->msg_write_idx < dev->msgs_num; dev-
> >msg_write_idx++) {
> + u32 flags = msgs[dev->msg_write_idx].flags;
+ empty line.
> /*
> * if target address has changed, we need to
> * reprogram the target address in the i2c
> @@ -588,8 +589,15 @@ static void i2c_dw_xfer_init(struct dw_i2c_dev
> *dev)
> * detected from the registers so we set it
> always
> * when writing/reading the last byte.
> */
> +
> + /*
> + * i2c-core.c always set the buffer length of
set -> sets
> + * I2C_FUNC_SMBUS_BLOCK_DATA to 1. The length
> will
> + * be adjusted when receiving the first byte.
> + * Thus we can't stop the transaction here.
> + */
>
> @@ -635,6 +648,25 @@ static void i2c_dw_xfer_init(struct dw_i2c_dev
> *dev)
> dw_writel(dev, intr_mask, DW_IC_INTR_MASK);
> }
>
> +static u8
> +i2c_dw_recv_len(struct dw_i2c_dev *dev, u8 len)
> +{
> + struct i2c_msg *msgs = dev->msgs;
> + u32 flags = msgs[dev->msg_read_idx].flags;
> +
> + /*
> + * Adjust the buffer length and mask the flag
> + * after receiving the first byte
Add dot to the end, please.
> + */
> + len = (flags & I2C_CLIENT_PEC) ? len + 2 : len + 1;
len += flags & I2C_CLIENT_PEC ? 2 : 1;
> + dev->tx_buf_len = len > dev->rx_outstanding ?
> + len - dev->rx_outstanding : 0;
Can be len more than twice longer as rx_outstanding?
Would it be better to write as
tx_buf_len = len - min(len, rx_outstanding);
?
> + msgs[dev->msg_read_idx].len = len;
> + msgs[dev->msg_read_idx].flags &= ~I2C_M_RECV_LEN;
> +
> + return len;
> +}
> +
> static void
> i2c_dw_read(struct dw_i2c_dev *dev)
> {
> @@ -659,7 +691,14 @@ static void i2c_dw_xfer_init(struct dw_i2c_dev
> *dev)
> rx_valid = dw_readl(dev, DW_IC_RXFLR);
>
> for (; len > 0 && rx_valid > 0; len--, rx_valid--) {
> - *buf++ = dw_readl(dev, DW_IC_DATA_CMD);
> + u32 flags = msgs[dev->msg_read_idx].flags;
+ empty line.
> + *buf = dw_readl(dev, DW_IC_DATA_CMD);
> + /* Ensure length byte is a valid value */
> + if (flags & I2C_M_RECV_LEN &&
> + *buf <= I2C_SMBUS_BLOCK_MAX && *buf >
> 0) {
Is it my mail client or indentation is wrong?
> + len = i2c_dw_recv_len(dev, *buf);
> + }
> + buf++;
> dev->rx_outstanding--;
> }
--
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy
^ permalink raw reply
* Re: [PATCH v5 6/6] i2c: use an IRQ to report Host Notify events, not alert
From: Benjamin Tissoires @ 2016-11-07 16:18 UTC (permalink / raw)
To: Wolfram Sang; +Cc: Dmitry Torokhov, linux-i2c, linux-kernel, Jean Delvare
In-Reply-To: <20161107002034.GB1442@katana>
On Nov 07 2016 or thereabouts, Wolfram Sang wrote:
> On Thu, Oct 13, 2016 at 02:10:40PM +0200, Benjamin Tissoires wrote:
> > The current SMBus Host Notify implementation relies on .alert() to
> > relay its notifications. However, the use cases where SMBus Host
> > Notify is needed currently is to signal data ready on touchpads.
> >
> > This is closer to an IRQ than a custom API through .alert().
> > Given that the 2 touchpad manufacturers (Synaptics and Elan) that
> > use SMBus Host Notify don't put any data in the SMBus payload, the
> > concept actually matches one to one.
>
> I see the advantages. The only question I have: What if we encounter
> devices in the future which do put data in the payload? Can this
> mechanism be extended to handle that?
I had this very same problem when dealing with hid-rmi. In that case, we
have an interrupt that contains data.
I somewhat solved this by the following commit transposed in the hid-rmi
world:
https://github.com/bentiss/linux/commit/d24d938e1eabba026c3c5e4daeee3a16403295de
The idea is the following:
- we extend the internal call of i2c_handle_smbus_host_notify() by
re-adding a data payload.
- this call takes a spinlock, and copy the data into an internal
placeholder, releases the spinlock and then triggers the irq
- when the client gets an interrupt, it calls
i2c_retrieve_host_notify_data() which is protected by the spinlock and
returns the *current* data
The only difficulty for the client now is that it might access newer
data than the origin interrupt. This can be solved by 2 solutions. Either
we use a kfifo in i2c_core, to the risk of having some complex processing.
Either we just let the client dealing with that by implementing itself
the kfifo. The data just needs to be retrieved in the non threaded part
of the interrupt handler, and stored by the client itself.
So all in all, I think adding the data will be just adding a spinlock, a
placeholder and a new internal API to retrieve it.
I am not sure we have other means of extending the IRQ processing, but I
am open to suggestions.
Cheers,
Benjamin
>
> >
> > Benefits are multiple:
> > - simpler code and API: the client will just have an IRQ, and
> > nothing needs to be added in the adapter beside internally
> > enabling it.
> > - no more specific workqueue, the threading is handled by IRQ core
> > directly (when required)
> > - no more races when removing the device (the drivers are already
> > required to disable irq on remove)
> > - simpler handling for drivers: use plain regular IRQs
> > - no more dependency on i2c-smbus for i2c-i801 (and any other adapter)
> > - the IRQ domain is created automatically when the adapter exports
> > the Host Notify capability
> > - the IRQ are assign only if ACPI, OF and the caller did not assign
> > one already
> > - the domain is automatically destroyed on remove
> > - fewer lines of code (minus 20, yeah!)
> >
> > Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
>
> Thanks for keeping at it!
>
^ permalink raw reply
* Re: [PATCHv7 07/11] i2c: match dt-style device names from sysfs interface
From: Javier Martinez Canillas @ 2016-11-07 19:10 UTC (permalink / raw)
To: Kieran Bingham, Wolfram Sang, Lee Jones; +Cc: linux-i2c, linux-kernel, sameo
In-Reply-To: <1478522866-29620-8-git-send-email-kieran@bingham.xyz>
Hello Kieran,
On 11/07/2016 09:47 AM, Kieran Bingham wrote:
> A user can choose to instantiate a device on an i2c bus using the sysfs
> interface by providing a string and address to match and communicate
> with the device on the bus. Presently this string is only matched
> against the old i2c device id style strings, even in the presence of
> full device tree compatible strings with vendor prefixes.
>
> Providing a vendor-prefixed string to the sysfs interface will not match
> against the device tree of_match_device() calls as there is no device
> tree node to parse from the sysfs interface.
>
> Convert i2c_of_match_device_strip_vendor() such that it can match both
> vendor prefixed and stripped compatible strings on the sysfs interface.
>
> Signed-off-by: Kieran Bingham <kieran@bingham.xyz>
>
Patch looks good to me.
Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com>
Best regards,
--
Javier Martinez Canillas
Open Source Group
Samsung Research America
^ permalink raw reply
* Re: [PATCHv7 09/11] mfd: 88pm860x: Move over to new I2C device .probe() call
From: Javier Martinez Canillas @ 2016-11-07 19:13 UTC (permalink / raw)
To: Kieran Bingham, Wolfram Sang, Lee Jones; +Cc: linux-i2c, linux-kernel, sameo
In-Reply-To: <1478522866-29620-10-git-send-email-kieran@bingham.xyz>
Hello Kieran,
On 11/07/2016 09:47 AM, Kieran Bingham wrote:
> From: Lee Jones <lee.jones@linaro.org>
>
> As part of an effort to rid the mostly unused second parameter for I2C
> related .probe() functions and to conform to other existing frameworks
> we're moving over to a temporary replacement .probe() call-back.
>
> Acked-by: Grant Likely <grant.likely@linaro.org>
> Signed-off-by: Lee Jones <lee.jones@linaro.org>
> Signed-off-by: Kieran Bingham <kieran@bingham.xyz>
>
Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com>
Best regards,
--
Javier Martinez Canillas
Open Source Group
Samsung Research America
^ permalink raw reply
* Re: [PATCHv7 10/11] mfd: as3722: Rid driver of superfluous I2C device ID structure
From: Javier Martinez Canillas @ 2016-11-07 19:20 UTC (permalink / raw)
To: Kieran Bingham, Wolfram Sang, Lee Jones; +Cc: linux-i2c, linux-kernel, sameo
In-Reply-To: <1478522866-29620-11-git-send-email-kieran@bingham.xyz>
Hello Kieran,
On 11/07/2016 09:47 AM, Kieran Bingham wrote:
> From: Lee Jones <lee.jones@linaro.org>
>
> Also remove unused second probe() parameter 'i2c_device_id'.
>
> Acked-by: Grant Likely <grant.likely@linaro.org>
> Signed-off-by: Lee Jones <lee.jones@linaro.org>
> Signed-off-by: Kieran Bingham <kieran@bingham.xyz>
>
> ---
> Changes since v4
> - Rename .probe2 to probe_new
>
> drivers/mfd/as3722.c | 12 ++----------
> 1 file changed, 2 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/mfd/as3722.c b/drivers/mfd/as3722.c
> index f87342c211bc..7d8c5e4136e2 100644
> --- a/drivers/mfd/as3722.c
> +++ b/drivers/mfd/as3722.c
> @@ -354,8 +354,7 @@ static int as3722_i2c_of_probe(struct i2c_client *i2c,
> return 0;
> }
>
> -static int as3722_i2c_probe(struct i2c_client *i2c,
> - const struct i2c_device_id *id)
> +static int as3722_i2c_probe(struct i2c_client *i2c)
> {
This is OK...
> struct as3722 *as3722;
> unsigned long irq_flags;
> @@ -441,12 +440,6 @@ static const struct of_device_id as3722_of_match[] = {
> };
> MODULE_DEVICE_TABLE(of, as3722_of_match);
>
> -static const struct i2c_device_id as3722_i2c_id[] = {
> - { "as3722", 0 },
> - {},
> -};
> -MODULE_DEVICE_TABLE(i2c, as3722_i2c_id);
> -
... but I don't think this is correct. Otherwise you will break module
autoload for this driver since modpost needs the I2C device ID table
info to fill the i2c modalias in the drivers' module.
Remember that i2c_device_uevent() always reports modalias of the form
MODALIAS=i2c:<foo> even when your series allows to match without a I2C
device ID table.
Best regards,
--
Javier Martinez Canillas
Open Source Group
Samsung Research America
^ permalink raw reply
* Re: [PATCHv7 08/11] i2c: match vendorless strings on the internal string length
From: Javier Martinez Canillas @ 2016-11-07 19:12 UTC (permalink / raw)
To: Kieran Bingham, Wolfram Sang, Lee Jones; +Cc: linux-i2c, linux-kernel, sameo
In-Reply-To: <1478522866-29620-9-git-send-email-kieran@bingham.xyz>
Hello Kieran,
On 11/07/2016 09:47 AM, Kieran Bingham wrote:
> If a user provides a shortened string to match a device to the sysfs i2c
> interface it will match on the first string that contains that string
> prefix.
>
> for example:
> echo a 0x68 > /sys/bus/i2c/devices/i2c-2/new_device
> will match as3711, as3722, and ak8975 incorrectly.
>
> Correct this by using sysfs_streq to match the string exactly
>
> Signed-off-by: Kieran Bingham <kieran@bingham.xyz>
>
Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com>
Best regards,
--
Javier Martinez Canillas
Open Source Group
Samsung Research America
^ permalink raw reply
* [PATCH 1/2] i2c: octeon: Fix register access
From: Paul Burton @ 2016-11-07 20:09 UTC (permalink / raw)
To: linux-i2c; +Cc: linux-mips, Paul Burton, Jan Glauber, David Daney, Wolfram Sang
Commit 70121f7f3725 ("i2c: octeon: thunderx: Limit register access
retries") attempted to replace potentially infinite loops with ones
which will time out using readq_poll_timeout, but in doing so it
inverted the condition for exiting this loop.
Tested on a Rhino Labs UTM-8 with Octeon CN7130.
Signed-off-by: Paul Burton <paul.burton@imgtec.com>
Cc: Jan Glauber <jglauber@cavium.com>
Cc: David Daney <david.daney@cavium.com>
Cc: Wolfram Sang <wsa@the-dreams.de>
Cc: linux-i2c@vger.kernel.org
---
drivers/i2c/busses/i2c-octeon-core.h | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/i2c/busses/i2c-octeon-core.h b/drivers/i2c/busses/i2c-octeon-core.h
index 1db7c83..d980406 100644
--- a/drivers/i2c/busses/i2c-octeon-core.h
+++ b/drivers/i2c/busses/i2c-octeon-core.h
@@ -146,8 +146,9 @@ static inline void octeon_i2c_reg_write(struct octeon_i2c *i2c, u64 eop_reg, u8
__raw_writeq(SW_TWSI_V | eop_reg | data, i2c->twsi_base + SW_TWSI(i2c));
- readq_poll_timeout(i2c->twsi_base + SW_TWSI(i2c), tmp, tmp & SW_TWSI_V,
- I2C_OCTEON_EVENT_WAIT, i2c->adap.timeout);
+ readq_poll_timeout(i2c->twsi_base + SW_TWSI(i2c), tmp,
+ !(tmp & SW_TWSI_V), I2C_OCTEON_EVENT_WAIT,
+ i2c->adap.timeout);
}
#define octeon_i2c_ctl_write(i2c, val) \
@@ -173,7 +174,7 @@ static inline int octeon_i2c_reg_read(struct octeon_i2c *i2c, u64 eop_reg,
__raw_writeq(SW_TWSI_V | eop_reg | SW_TWSI_R, i2c->twsi_base + SW_TWSI(i2c));
ret = readq_poll_timeout(i2c->twsi_base + SW_TWSI(i2c), tmp,
- tmp & SW_TWSI_V, I2C_OCTEON_EVENT_WAIT,
+ !(tmp & SW_TWSI_V), I2C_OCTEON_EVENT_WAIT,
i2c->adap.timeout);
if (error)
*error = ret;
--
2.10.2
^ permalink raw reply related
* [PATCH 2/2] i2c: octeon: Fix waiting for operation completion
From: Paul Burton @ 2016-11-07 20:09 UTC (permalink / raw)
To: linux-i2c
Cc: linux-mips, Paul Burton, David Daney, Jan Glauber, Peter Swain,
Wolfram Sang
In-Reply-To: <20161107200921.30284-1-paul.burton@imgtec.com>
Commit 1bb1ff3e7c74 ("i2c: octeon: Improve performance if interrupt is
early") modified octeon_i2c_wait() & octeon_i2c_hlc_wait() to attempt to
check for a valid bit being clear & if not to sleep for a while then try
again before waiting on a waitqueue which may time out. However it does
so by sleeping within a function called as the condition provided to
wait_event_timeout() which seems to cause strange behaviour, with the
system hanging during boot with the condition being checked constantly &
the timeout not seeming to have any effect.
Fix this by instead checking for the valid bit being clear in the
octeon_i2c(_hlc)_wait() functions & sleeping there if that condition is
not met, then calling the wait_event_timeout with a condition that does
not sleep.
Tested on a Rhino Labs UTM-8 with Octeon CN7130.
Signed-off-by: Paul Burton <paul.burton@imgtec.com>
Cc: David Daney <david.daney@cavium.com>
Cc: Jan Glauber <jglauber@cavium.com>
Cc: Peter Swain <pswain@cavium.com>
Cc: Wolfram Sang <wsa@the-dreams.de>
Cc: linux-i2c@vger.kernel.org
---
drivers/i2c/busses/i2c-octeon-core.c | 58 ++++++++++--------------------------
1 file changed, 15 insertions(+), 43 deletions(-)
diff --git a/drivers/i2c/busses/i2c-octeon-core.c b/drivers/i2c/busses/i2c-octeon-core.c
index 419b54b..2e270a1 100644
--- a/drivers/i2c/busses/i2c-octeon-core.c
+++ b/drivers/i2c/busses/i2c-octeon-core.c
@@ -36,24 +36,6 @@ static bool octeon_i2c_test_iflg(struct octeon_i2c *i2c)
return (octeon_i2c_ctl_read(i2c) & TWSI_CTL_IFLG);
}
-static bool octeon_i2c_test_ready(struct octeon_i2c *i2c, bool *first)
-{
- if (octeon_i2c_test_iflg(i2c))
- return true;
-
- if (*first) {
- *first = false;
- return false;
- }
-
- /*
- * IRQ has signaled an event but IFLG hasn't changed.
- * Sleep and retry once.
- */
- usleep_range(I2C_OCTEON_EVENT_WAIT, 2 * I2C_OCTEON_EVENT_WAIT);
- return octeon_i2c_test_iflg(i2c);
-}
-
/**
* octeon_i2c_wait - wait for the IFLG to be set
* @i2c: The struct octeon_i2c
@@ -80,8 +62,13 @@ static int octeon_i2c_wait(struct octeon_i2c *i2c)
}
i2c->int_enable(i2c);
- time_left = wait_event_timeout(i2c->queue, octeon_i2c_test_ready(i2c, &first),
- i2c->adap.timeout);
+ time_left = i2c->adap.timeout;
+ if (!octeon_i2c_test_iflg(i2c)) {
+ usleep_range(I2C_OCTEON_EVENT_WAIT, 2 * I2C_OCTEON_EVENT_WAIT);
+ time_left = wait_event_timeout(i2c->queue,
+ octeon_i2c_test_iflg(i2c),
+ time_left);
+ }
i2c->int_disable(i2c);
if (i2c->broken_irq_check && !time_left &&
@@ -99,26 +86,8 @@ static int octeon_i2c_wait(struct octeon_i2c *i2c)
static bool octeon_i2c_hlc_test_valid(struct octeon_i2c *i2c)
{
- return (__raw_readq(i2c->twsi_base + SW_TWSI(i2c)) & SW_TWSI_V) == 0;
-}
-
-static bool octeon_i2c_hlc_test_ready(struct octeon_i2c *i2c, bool *first)
-{
/* check if valid bit is cleared */
- if (octeon_i2c_hlc_test_valid(i2c))
- return true;
-
- if (*first) {
- *first = false;
- return false;
- }
-
- /*
- * IRQ has signaled an event but valid bit isn't cleared.
- * Sleep and retry once.
- */
- usleep_range(I2C_OCTEON_EVENT_WAIT, 2 * I2C_OCTEON_EVENT_WAIT);
- return octeon_i2c_hlc_test_valid(i2c);
+ return (__raw_readq(i2c->twsi_base + SW_TWSI(i2c)) & SW_TWSI_V) == 0;
}
static void octeon_i2c_hlc_int_clear(struct octeon_i2c *i2c)
@@ -176,7 +145,6 @@ static void octeon_i2c_hlc_disable(struct octeon_i2c *i2c)
*/
static int octeon_i2c_hlc_wait(struct octeon_i2c *i2c)
{
- bool first = true;
int time_left;
/*
@@ -194,9 +162,13 @@ static int octeon_i2c_hlc_wait(struct octeon_i2c *i2c)
}
i2c->hlc_int_enable(i2c);
- time_left = wait_event_timeout(i2c->queue,
- octeon_i2c_hlc_test_ready(i2c, &first),
- i2c->adap.timeout);
+ time_left = i2c->adap.timeout;
+ if (!octeon_i2c_hlc_test_valid(i2c)) {
+ usleep_range(I2C_OCTEON_EVENT_WAIT, 2 * I2C_OCTEON_EVENT_WAIT);
+ time_left = wait_event_timeout(i2c->queue,
+ octeon_i2c_hlc_test_valid(i2c),
+ time_left);
+ }
i2c->hlc_int_disable(i2c);
if (!time_left)
octeon_i2c_hlc_int_clear(i2c);
--
2.10.2
^ permalink raw reply related
* Re: [v15, 3/7] powerpc/fsl: move mpc85xx.h to include/linux/fsl
From: Arnd Bergmann @ 2016-11-07 21:20 UTC (permalink / raw)
To: Y.B. Lu
Cc: linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org,
linux-mmc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
ulf.hansson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org, Scott Wood,
Mark Rutland, Greg Kroah-Hartman, X.B. Xie, M.H. Lian,
linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-clk-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Qiang Zhao,
Russell King, Bhupesh Sharma, Joerg Roedel, Claudiu Manoil,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Rob Herring
In-Reply-To: <DB6PR0401MB2536B1B2E6AC36BDC8FC540DF8AE0-2mNvjAGDOPkZcyyZo0JLBI3W/0Ik+aLCnBOFsp37pqbUKgpGm//BTAC/G2K4zDHf@public.gmane.org>
On Monday, October 31, 2016 9:35:33 AM CET Y.B. Lu wrote:
> >
> > I don't see any of the contents of this header referenced by the soc
> > driver any more. I think you can just drop this patch.
> >
>
> [Lu Yangbo-B47093] This header file was included by guts.c.
> The guts driver used macro SVR_MAJ/SVR_MIN for calculation.
>
> This header file was for powerpc arch before. And this patch is to made it as
> common header file for both ARM and PPC.
> Sooner or later this is needed.
Let's discuss it once we actually need the header then, ok?
Arnd
--
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
* Re: [PATCHv7 10/11] mfd: as3722: Rid driver of superfluous I2C device ID structure
From: Kieran Bingham @ 2016-11-07 22:05 UTC (permalink / raw)
To: Javier Martinez Canillas, Wolfram Sang, Lee Jones
Cc: linux-i2c, linux-kernel, sameo
In-Reply-To: <1ac3a33e-a85c-e781-81b6-c39f3d533da8@osg.samsung.com>
Thanks for your reviews again Javier,
On 07/11/16 19:20, Javier Martinez Canillas wrote:
> Hello Kieran,
>
> On 11/07/2016 09:47 AM, Kieran Bingham wrote:
>> From: Lee Jones <lee.jones@linaro.org>
>>
>> Also remove unused second probe() parameter 'i2c_device_id'.
>>
>> Acked-by: Grant Likely <grant.likely@linaro.org>
>> Signed-off-by: Lee Jones <lee.jones@linaro.org>
>> Signed-off-by: Kieran Bingham <kieran@bingham.xyz>
>>
>> ---
>> Changes since v4
>> - Rename .probe2 to probe_new
>>
>> drivers/mfd/as3722.c | 12 ++----------
>> 1 file changed, 2 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/mfd/as3722.c b/drivers/mfd/as3722.c
>> index f87342c211bc..7d8c5e4136e2 100644
>> --- a/drivers/mfd/as3722.c
>> +++ b/drivers/mfd/as3722.c
>> @@ -354,8 +354,7 @@ static int as3722_i2c_of_probe(struct i2c_client *i2c,
>> return 0;
>> }
>>
>> -static int as3722_i2c_probe(struct i2c_client *i2c,
>> - const struct i2c_device_id *id)
>> +static int as3722_i2c_probe(struct i2c_client *i2c)
>> {
>
> This is OK...
>
>> struct as3722 *as3722;
>> unsigned long irq_flags;
>> @@ -441,12 +440,6 @@ static const struct of_device_id as3722_of_match[] = {
>> };
>> MODULE_DEVICE_TABLE(of, as3722_of_match);
>>
>> -static const struct i2c_device_id as3722_i2c_id[] = {
>> - { "as3722", 0 },
>> - {},
>> -};
>> -MODULE_DEVICE_TABLE(i2c, as3722_i2c_id);
>> -
>
> ... but I don't think this is correct. Otherwise you will break module
> autoload for this driver since modpost needs the I2C device ID table
> info to fill the i2c modalias in the drivers' module.
>
> Remember that i2c_device_uevent() always reports modalias of the form
> MODALIAS=i2c:<foo> even when your series allows to match without a I2C
> device ID table.
>
Ok - Thanks for the reminder. I'll try to bear this in mind when we
start updating drivers.
For now we can consider this patch dropped from the series I think.
> Best regards,
>
--
Regards
Kieran Bingham
^ permalink raw reply
* Re: [PATCHv7 10/11] mfd: as3722: Rid driver of superfluous I2C device ID structure
From: Wolfram Sang @ 2016-11-07 23:09 UTC (permalink / raw)
To: Javier Martinez Canillas
Cc: Kieran Bingham, Lee Jones, linux-i2c, linux-kernel, sameo
In-Reply-To: <1ac3a33e-a85c-e781-81b6-c39f3d533da8@osg.samsung.com>
[-- Attachment #1: Type: text/plain, Size: 357 bytes --]
> Remember that i2c_device_uevent() always reports modalias of the form
> MODALIAS=i2c:<foo> even when your series allows to match without a I2C
> device ID table.
Not always. Can't we do something similar like ACPI does with
acpi_device_uevent_modalias()?
I mean the whole point of this series is to remove the need of having an
I2C device ID table...
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [Linux-parport] True Parallel Port Interface for Bit-banging?
From: Maciej S. Szmigiero @ 2016-11-07 23:53 UTC (permalink / raw)
To: Sebastian Frias
Cc: Wolfram Sang, Sudip Mukherjee, linux-parport, philb, tim,
Jean Delvare, linux-i2c
In-Reply-To: <58207746.2020802@laposte.net>
On 07.11.2016 13:44, Sebastian Frias wrote:
> On 10/29/2016 01:12 AM, Maciej S. Szmigiero wrote:
>>> Would you mind sharing the laptop's model?
>>
>> This is Elitebook model 8570p.
>
> That seems like a fairly recent laptop:
>
> http://www.notebookcheck.net/Review-HP-EliteBook-8570p-B6Q03EA-ABD-Notebook.82291.0.html
>
> and it has a true parallel port?
It has parallel port on address 0x378/0x778, IRQ 5 by default which is
realized by Super IO chip.
In PnP config also available for selection are addresses 0x278/0x678,
0x3bc/0x7bc, IRQ 7 and ISA DMA channels 1 and 3.
Although I have never used this port personally it looks like what can
be considered a "true" parallel port.
> Thanks a lot for the pointer!
You are welcome.
Maciej
^ permalink raw reply
* Re: [PATCHv7 10/11] mfd: as3722: Rid driver of superfluous I2C device ID structure
From: Javier Martinez Canillas @ 2016-11-08 2:02 UTC (permalink / raw)
To: Kieran Bingham, Wolfram Sang, Lee Jones; +Cc: linux-i2c, linux-kernel, sameo
In-Reply-To: <fee215fd-bdd4-9657-92db-1f77794b1abf@bingham.xyz>
Hello Kieran,
On 11/07/2016 07:05 PM, Kieran Bingham wrote:
> Thanks for your reviews again Javier,
>
Thanks to you for keep pushing this series.
> On 07/11/16 19:20, Javier Martinez Canillas wrote:
>> Hello Kieran,
>>
>> On 11/07/2016 09:47 AM, Kieran Bingham wrote:
>>> From: Lee Jones <lee.jones@linaro.org>
>>>
>>> Also remove unused second probe() parameter 'i2c_device_id'.
>>>
>>> Acked-by: Grant Likely <grant.likely@linaro.org>
>>> Signed-off-by: Lee Jones <lee.jones@linaro.org>
>>> Signed-off-by: Kieran Bingham <kieran@bingham.xyz>
>>>
>>> ---
>>> Changes since v4
>>> - Rename .probe2 to probe_new
>>>
>>> drivers/mfd/as3722.c | 12 ++----------
>>> 1 file changed, 2 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/drivers/mfd/as3722.c b/drivers/mfd/as3722.c
>>> index f87342c211bc..7d8c5e4136e2 100644
>>> --- a/drivers/mfd/as3722.c
>>> +++ b/drivers/mfd/as3722.c
>>> @@ -354,8 +354,7 @@ static int as3722_i2c_of_probe(struct i2c_client *i2c,
>>> return 0;
>>> }
>>>
>>> -static int as3722_i2c_probe(struct i2c_client *i2c,
>>> - const struct i2c_device_id *id)
>>> +static int as3722_i2c_probe(struct i2c_client *i2c)
>>> {
>>
>> This is OK...
>>
>>> struct as3722 *as3722;
>>> unsigned long irq_flags;
>>> @@ -441,12 +440,6 @@ static const struct of_device_id as3722_of_match[] = {
>>> };
>>> MODULE_DEVICE_TABLE(of, as3722_of_match);
>>>
>>> -static const struct i2c_device_id as3722_i2c_id[] = {
>>> - { "as3722", 0 },
>>> - {},
>>> -};
>>> -MODULE_DEVICE_TABLE(i2c, as3722_i2c_id);
>>> -
>>
>> ... but I don't think this is correct. Otherwise you will break module
>> autoload for this driver since modpost needs the I2C device ID table
>> info to fill the i2c modalias in the drivers' module.
>>
>> Remember that i2c_device_uevent() always reports modalias of the form
>> MODALIAS=i2c:<foo> even when your series allows to match without a I2C
>> device ID table.
>>
>
> Ok - Thanks for the reminder. I'll try to bear this in mind when we
> start updating drivers.
>
> For now we can consider this patch dropped from the series I think.
>
Yes, or you could just do the change that uses probe_new for now but
leave the MODULE_DEVICE_TABLE().
>> Best regards,
>>
>
Best regards,
--
Javier Martinez Canillas
Open Source Group
Samsung Research America
^ permalink raw reply
* Re: [PATCHv7 10/11] mfd: as3722: Rid driver of superfluous I2C device ID structure
From: Javier Martinez Canillas @ 2016-11-08 2:14 UTC (permalink / raw)
To: Wolfram Sang; +Cc: Kieran Bingham, Lee Jones, linux-i2c, linux-kernel, sameo
In-Reply-To: <20161107230931.GA1439@katana>
Hello Wolfram,
On 11/07/2016 08:09 PM, Wolfram Sang wrote:
>
>> Remember that i2c_device_uevent() always reports modalias of the form
>> MODALIAS=i2c:<foo> even when your series allows to match without a I2C
>> device ID table.
>
> Not always. Can't we do something similar like ACPI does with
Right, I meant that it always report a platform I2C modalias for both OF
and legacy platform data I2C device registration mechanisms.
> acpi_device_uevent_modalias()?
>
Yes, doing that is trivial. I posted a RFC patch about a year ago that
changes i2c_device_uevent() to report a proper OF modalias [0] as a part
of a series that fixed module autoload in a bunch of I2C drivers [1].
What's tricky is to make sure that the change won't introduce regressions
in current I2C drivers. I enumerated the possible issues if the I2C core
starts reporting OF modaliases and fixed some of them in the same series.
> I mean the whole point of this series is to remove the need of having an
> I2C device ID table...
>
Agreed, one of the issues was that the I2C table was needed anyways for
the core to match and pass a struct i2c_device_id to the probe's function.
This series solves that so once it lands, I plan to address the possible
issues in the I2C drivers and re-send [0] as a proper patch.
[0]: https://patchwork.kernel.org/patch/6903991/
[1]: https://lkml.org/lkml/2015/7/30/519
Best regards,
--
Javier Martinez Canillas
Open Source Group
Samsung Research America
^ permalink raw reply
* RE: [v15, 3/7] powerpc/fsl: move mpc85xx.h to include/linux/fsl
From: Y.B. Lu @ 2016-11-08 6:49 UTC (permalink / raw)
To: Arnd Bergmann
Cc: linuxppc-dev@lists.ozlabs.org, linux-mmc@vger.kernel.org,
ulf.hansson@linaro.org, Scott Wood, Mark Rutland,
Greg Kroah-Hartman, X.B. Xie, M.H. Lian,
linux-i2c@vger.kernel.org, linux-clk@vger.kernel.org, Qiang Zhao,
Russell King, Bhupesh Sharma, Joerg Roedel, Claudiu Manoil,
devicetree@vger.kernel.org, Rob Herring
In-Reply-To: <1543282.vWjeo4ynTa@wuerfel>
Hi Arnd,
> -----Original Message-----
> From: Arnd Bergmann [mailto:arnd@arndb.de]
> Sent: Tuesday, November 08, 2016 5:20 AM
> To: Y.B. Lu
> Cc: linuxppc-dev@lists.ozlabs.org; linux-mmc@vger.kernel.org;
> ulf.hansson@linaro.org; Scott Wood; Mark Rutland; Greg Kroah-Hartman; X.B.
> Xie; M.H. Lian; linux-i2c@vger.kernel.org; linux-clk@vger.kernel.org;
> Qiang Zhao; Russell King; Bhupesh Sharma; Joerg Roedel; Claudiu Manoil;
> devicetree@vger.kernel.org; Rob Herring; Santosh Shilimkar; linux-arm-
> kernel@lists.infradead.org; netdev@vger.kernel.org; linux-
> kernel@vger.kernel.org; Leo Li; iommu@lists.linux-foundation.org; Kumar
> Gala
> Subject: Re: [v15, 3/7] powerpc/fsl: move mpc85xx.h to include/linux/fsl
>
> On Monday, October 31, 2016 9:35:33 AM CET Y.B. Lu wrote:
> > >
> > > I don't see any of the contents of this header referenced by the soc
> > > driver any more. I think you can just drop this patch.
> > >
> >
> > [Lu Yangbo-B47093] This header file was included by guts.c.
> > The guts driver used macro SVR_MAJ/SVR_MIN for calculation.
> >
> > This header file was for powerpc arch before. And this patch is to
> > made it as common header file for both ARM and PPC.
> > Sooner or later this is needed.
>
> Let's discuss it once we actually need the header then, ok?
[Lu Yangbo-B47093] As I said, this header file was included by guts.c in patch 4.
The guts driver used macro SVR_MAJ/SVR_MIN for calculation which were defined in this header file.
Did you suggest we dropped this patch and just calculated them in driver?
Thanks :)
>
> Arnd
^ permalink raw reply
* Re: [PATCH 1/2] i2c: octeon: Fix register access
From: Jan Glauber @ 2016-11-08 7:13 UTC (permalink / raw)
To: Paul Burton; +Cc: linux-i2c, linux-mips, David Daney, Wolfram Sang, stable
In-Reply-To: <20161107200921.30284-1-paul.burton@imgtec.com>
On Mon, Nov 07, 2016 at 08:09:20PM +0000, Paul Burton wrote:
> Commit 70121f7f3725 ("i2c: octeon: thunderx: Limit register access
> retries") attempted to replace potentially infinite loops with ones
> which will time out using readq_poll_timeout, but in doing so it
> inverted the condition for exiting this loop.
>
> Tested on a Rhino Labs UTM-8 with Octeon CN7130.
>
> Signed-off-by: Paul Burton <paul.burton@imgtec.com>
> Cc: Jan Glauber <jglauber@cavium.com>
> Cc: David Daney <david.daney@cavium.com>
> Cc: Wolfram Sang <wsa@the-dreams.de>
> Cc: linux-i2c@vger.kernel.org
>
> ---
Acked-by: Jan Glauber <jglauber@cavium.com>
Thanks for spotting this. I think this should go into stable too for
4.8, so adding Cc: stable@vger.kernel.org.
> drivers/i2c/busses/i2c-octeon-core.h | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-octeon-core.h b/drivers/i2c/busses/i2c-octeon-core.h
> index 1db7c83..d980406 100644
> --- a/drivers/i2c/busses/i2c-octeon-core.h
> +++ b/drivers/i2c/busses/i2c-octeon-core.h
> @@ -146,8 +146,9 @@ static inline void octeon_i2c_reg_write(struct octeon_i2c *i2c, u64 eop_reg, u8
>
> __raw_writeq(SW_TWSI_V | eop_reg | data, i2c->twsi_base + SW_TWSI(i2c));
>
> - readq_poll_timeout(i2c->twsi_base + SW_TWSI(i2c), tmp, tmp & SW_TWSI_V,
> - I2C_OCTEON_EVENT_WAIT, i2c->adap.timeout);
> + readq_poll_timeout(i2c->twsi_base + SW_TWSI(i2c), tmp,
> + !(tmp & SW_TWSI_V), I2C_OCTEON_EVENT_WAIT,
> + i2c->adap.timeout);
> }
>
> #define octeon_i2c_ctl_write(i2c, val) \
> @@ -173,7 +174,7 @@ static inline int octeon_i2c_reg_read(struct octeon_i2c *i2c, u64 eop_reg,
> __raw_writeq(SW_TWSI_V | eop_reg | SW_TWSI_R, i2c->twsi_base + SW_TWSI(i2c));
>
> ret = readq_poll_timeout(i2c->twsi_base + SW_TWSI(i2c), tmp,
> - tmp & SW_TWSI_V, I2C_OCTEON_EVENT_WAIT,
> + !(tmp & SW_TWSI_V), I2C_OCTEON_EVENT_WAIT,
> i2c->adap.timeout);
> if (error)
> *error = ret;
> --
> 2.10.2
^ permalink raw reply
* Re: [PATCH 2/2] i2c: octeon: Fix waiting for operation completion
From: Jan Glauber @ 2016-11-08 9:20 UTC (permalink / raw)
To: Paul Burton; +Cc: linux-i2c, linux-mips, David Daney, Peter Swain, Wolfram Sang
In-Reply-To: <20161107200921.30284-2-paul.burton@imgtec.com>
On Mon, Nov 07, 2016 at 08:09:21PM +0000, Paul Burton wrote:
> Commit 1bb1ff3e7c74 ("i2c: octeon: Improve performance if interrupt is
> early") modified octeon_i2c_wait() & octeon_i2c_hlc_wait() to attempt to
> check for a valid bit being clear & if not to sleep for a while then try
> again before waiting on a waitqueue which may time out. However it does
> so by sleeping within a function called as the condition provided to
> wait_event_timeout() which seems to cause strange behaviour, with the
> system hanging during boot with the condition being checked constantly &
> the timeout not seeming to have any effect.
>
> Fix this by instead checking for the valid bit being clear in the
> octeon_i2c(_hlc)_wait() functions & sleeping there if that condition is
> not met, then calling the wait_event_timeout with a condition that does
> not sleep.
>
> Tested on a Rhino Labs UTM-8 with Octeon CN7130.
This patch breaks ipmi on ThunderX (CN88xx). We also have not seen the
problems you mention, although I must admit that I don't like the
complicated nested waits.
--Jan
> Signed-off-by: Paul Burton <paul.burton@imgtec.com>
> Cc: David Daney <david.daney@cavium.com>
> Cc: Jan Glauber <jglauber@cavium.com>
> Cc: Peter Swain <pswain@cavium.com>
> Cc: Wolfram Sang <wsa@the-dreams.de>
> Cc: linux-i2c@vger.kernel.org
> ---
>
> drivers/i2c/busses/i2c-octeon-core.c | 58 ++++++++++--------------------------
> 1 file changed, 15 insertions(+), 43 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-octeon-core.c b/drivers/i2c/busses/i2c-octeon-core.c
> index 419b54b..2e270a1 100644
> --- a/drivers/i2c/busses/i2c-octeon-core.c
> +++ b/drivers/i2c/busses/i2c-octeon-core.c
> @@ -36,24 +36,6 @@ static bool octeon_i2c_test_iflg(struct octeon_i2c *i2c)
> return (octeon_i2c_ctl_read(i2c) & TWSI_CTL_IFLG);
> }
>
> -static bool octeon_i2c_test_ready(struct octeon_i2c *i2c, bool *first)
> -{
> - if (octeon_i2c_test_iflg(i2c))
> - return true;
> -
> - if (*first) {
> - *first = false;
> - return false;
> - }
> -
> - /*
> - * IRQ has signaled an event but IFLG hasn't changed.
> - * Sleep and retry once.
> - */
> - usleep_range(I2C_OCTEON_EVENT_WAIT, 2 * I2C_OCTEON_EVENT_WAIT);
> - return octeon_i2c_test_iflg(i2c);
> -}
> -
> /**
> * octeon_i2c_wait - wait for the IFLG to be set
> * @i2c: The struct octeon_i2c
> @@ -80,8 +62,13 @@ static int octeon_i2c_wait(struct octeon_i2c *i2c)
> }
>
> i2c->int_enable(i2c);
> - time_left = wait_event_timeout(i2c->queue, octeon_i2c_test_ready(i2c, &first),
> - i2c->adap.timeout);
> + time_left = i2c->adap.timeout;
> + if (!octeon_i2c_test_iflg(i2c)) {
> + usleep_range(I2C_OCTEON_EVENT_WAIT, 2 * I2C_OCTEON_EVENT_WAIT);
> + time_left = wait_event_timeout(i2c->queue,
> + octeon_i2c_test_iflg(i2c),
> + time_left);
> + }
> i2c->int_disable(i2c);
>
> if (i2c->broken_irq_check && !time_left &&
> @@ -99,26 +86,8 @@ static int octeon_i2c_wait(struct octeon_i2c *i2c)
>
> static bool octeon_i2c_hlc_test_valid(struct octeon_i2c *i2c)
> {
> - return (__raw_readq(i2c->twsi_base + SW_TWSI(i2c)) & SW_TWSI_V) == 0;
> -}
> -
> -static bool octeon_i2c_hlc_test_ready(struct octeon_i2c *i2c, bool *first)
> -{
> /* check if valid bit is cleared */
> - if (octeon_i2c_hlc_test_valid(i2c))
> - return true;
> -
> - if (*first) {
> - *first = false;
> - return false;
> - }
> -
> - /*
> - * IRQ has signaled an event but valid bit isn't cleared.
> - * Sleep and retry once.
> - */
> - usleep_range(I2C_OCTEON_EVENT_WAIT, 2 * I2C_OCTEON_EVENT_WAIT);
> - return octeon_i2c_hlc_test_valid(i2c);
> + return (__raw_readq(i2c->twsi_base + SW_TWSI(i2c)) & SW_TWSI_V) == 0;
> }
>
> static void octeon_i2c_hlc_int_clear(struct octeon_i2c *i2c)
> @@ -176,7 +145,6 @@ static void octeon_i2c_hlc_disable(struct octeon_i2c *i2c)
> */
> static int octeon_i2c_hlc_wait(struct octeon_i2c *i2c)
> {
> - bool first = true;
> int time_left;
>
> /*
> @@ -194,9 +162,13 @@ static int octeon_i2c_hlc_wait(struct octeon_i2c *i2c)
> }
>
> i2c->hlc_int_enable(i2c);
> - time_left = wait_event_timeout(i2c->queue,
> - octeon_i2c_hlc_test_ready(i2c, &first),
> - i2c->adap.timeout);
> + time_left = i2c->adap.timeout;
> + if (!octeon_i2c_hlc_test_valid(i2c)) {
> + usleep_range(I2C_OCTEON_EVENT_WAIT, 2 * I2C_OCTEON_EVENT_WAIT);
> + time_left = wait_event_timeout(i2c->queue,
> + octeon_i2c_hlc_test_valid(i2c),
> + time_left);
> + }
> i2c->hlc_int_disable(i2c);
> if (!time_left)
> octeon_i2c_hlc_int_clear(i2c);
> --
> 2.10.2
^ permalink raw reply
* amd756 driver error introduced with kernel-4.9.0-0.rc1.git0.2.fc26
From: Johnny Bieren @ 2016-11-08 12:50 UTC (permalink / raw)
To: jdelvare; +Cc: linux-i2c
Hello,
It appears that a driver_probe_device failure was introduced with
kernel-4.9.0-0.rc1.git0.2.fc26. When booting this kernel, or any newer
kernel, I see the following:
[ 15.675485] ------------[ cut here ]------------
[ 15.680129] WARNING: CPU: 4 PID: 528 at drivers/base/dd.c:343
driver_probe_device+0x353/0x430
[ 15.688678] Modules linked in: i2c_amd756(+) pcc_cpufreq(-) k8temp
fjes amd_rng acpi_cpufreq tpm_tis tpm_tis_core tpm nfsd auth_rpcgss
nfs_acl lockd grace sunrpc xfs libcrc32c hpsa scsi_transport_sas
ata_generic pata_acpi serio_raw cciss pata_amd
[ 15.688681] CPU: 4 PID: 528 Comm: systemd-udevd Not tainted
4.9.0-0.rc1.git0.2.fc26.x86_64 #1
[ 15.688690] Hardware name: HP ProLiant DL385 G1, BIOS A05 03/01/2006
[ 15.688693] ffffb5ddc062bb00 ffffffff953e93fd 0000000000000000
0000000000000000
[ 15.688695] ffffb5ddc062bb40 ffffffff950a181b 00000157bd43a0a0
ffff8f70bd43a0a0
[ 15.688697] ffffffffc0440468 ffffffffc0440468 000000000000000b
0000000000000001
[ 15.688698] Call Trace:
[ 15.688703] [<ffffffff953e93fd>] dump_stack+0x63/0x86
[ 15.688706] [<ffffffff950a181b>] __warn+0xcb/0xf0
[ 15.688708] [<ffffffff950a194d>] warn_slowpath_null+0x1d/0x20
[ 15.688709] [<ffffffff955409a3>] driver_probe_device+0x353/0x430
[ 15.688711] [<ffffffff95540b5f>] __driver_attach+0xdf/0xf0
[ 15.688713] [<ffffffff95540a80>] ? driver_probe_device+0x430/0x430
[ 15.688715] [<ffffffff9553e25c>] bus_for_each_dev+0x6c/0xc0
[ 15.688717] [<ffffffff9553ffbe>] driver_attach+0x1e/0x20
[ 15.688718] [<ffffffff9553f9f0>] bus_add_driver+0x170/0x270
[ 15.688720] [<ffffffffc0407000>] ? 0xffffffffc0407000
[ 15.688722] [<ffffffff95541480>] driver_register+0x60/0xe0
[ 15.688723] [<ffffffffc0407000>] ? 0xffffffffc0407000
[ 15.688725] [<ffffffff9543e7dc>] __pci_register_driver+0x4c/0x50
[ 15.688728] [<ffffffffc040701e>] amd756_driver_init+0x1e/0x1000
[i2c_amd756]
[ 15.688731] [<ffffffff95002190>] do_one_initcall+0x50/0x180
[ 15.688734] [<ffffffff9520ab81>] ? __vunmap+0x81/0xd0
[ 15.688737] [<ffffffff9522ad29>] ? kmem_cache_alloc_trace+0x99/0x1b0
[ 15.688739] [<ffffffff951bb9aa>] ? do_init_module+0x27/0x1ef
[ 15.688741] [<ffffffff951bb9e2>] do_init_module+0x5f/0x1ef
[ 15.688744] [<ffffffff9512f581>] load_module+0x25b1/0x2980
[ 15.688746] [<ffffffff9512be60>] ? __symbol_put+0x60/0x60
[ 15.688749] [<ffffffff9512fac3>] SYSC_init_module+0x173/0x190
[ 15.688751] [<ffffffff9512fbfe>] SyS_init_module+0xe/0x10
[ 15.688753] [<ffffffff95003c17>] do_syscall_64+0x67/0x180
[ 15.688757] [<ffffffff958112ab>] entry_SYSCALL64_slow_path+0x25/0x25
[ 15.688758] ---[ end trace 9920abb49d6753e2 ]---
The problem does not appear in kernel-4.9.0-0.rc0.git9.1.fc26, but it
does appear in kernel-4.9.0-0.rc1.git0.2.fc26. It reproduces 100% of
the time for me. Can you please look into this issue?
Thanks,
Johnny Bieren
^ permalink raw reply
* Re: amd756 driver error introduced with kernel-4.9.0-0.rc1.git0.2.fc26
From: Jean Delvare @ 2016-11-08 13:58 UTC (permalink / raw)
To: Johnny Bieren; +Cc: linux-i2c
In-Reply-To: <5821CA1A.5020608@redhat.com>
Hi Johnny,
On Tue, 8 Nov 2016 07:50:34 -0500, Johnny Bieren wrote:
> Hello,
> It appears that a driver_probe_device failure was introduced with
> kernel-4.9.0-0.rc1.git0.2.fc26. When booting this kernel, or any newer
> kernel, I see the following:
>
> [ 15.675485] ------------[ cut here ]------------
> [ 15.680129] WARNING: CPU: 4 PID: 528 at drivers/base/dd.c:343
> driver_probe_device+0x353/0x430
> [ 15.688678] Modules linked in: i2c_amd756(+) pcc_cpufreq(-) k8temp
> (...)
>
> The problem does not appear in kernel-4.9.0-0.rc0.git9.1.fc26, but it
> does appear in kernel-4.9.0-0.rc1.git0.2.fc26. It reproduces 100% of
> the time for me. Can you please look into this issue?
Are the failing kernels built with option DEBUG_TEST_DRIVER_REMOVE
enabled?
--
Jean Delvare
SUSE L3 Support
^ permalink raw reply
* Re: [PATCH v2 3/4] Device bindings documentation updated ACPI-enabled platforms not currently supported
From: Luis Oliveira @ 2016-11-08 14:18 UTC (permalink / raw)
To: Andy Shevchenko, Luis Oliveira, Wolfram Sang, Ramiro Oliveira
Cc: Mark Rutland, jarkko.nikula, mika.westerberg, linux-i2c,
linux-kernel, robh+dt, devicetree, CARLOS.PALMINHA
In-Reply-To: <1477047272.6423.6.camel@linux.intel.com>
Hi,
As you suggested I will split the drivers. I am thinking of doing 5 patches:
- factor out master() parts
- separate Master part to i2c-designware-master.c (changes in i2c-designware-core.c)
- enable Slave part to i2c-designware-slave (changes in i2c-designware-core.c)
- glue drivers and device bindings
- cleaning
Regards,
Luis
On 21-Oct-16 11:54, Andy Shevchenko wrote:
> On Fri, 2016-10-21 at 10:56 +0100, Luis Oliveira wrote:
>> Since practically 90% of the code is shared between master and slave,
>> I was
>> thinking if it will be acceptable to use the same driver for both but
>> differentiate the master/slave mode by the compatible strings.
> It might be possible to split like other drivers do:
>
> 1. Core part (i2c-designware-core.c)
> 2. Master part (i2c-designware-master.c)
> 3. Slave part (i2c-designware-slave.c)
> 4. Glue drivers (like: i2c-designware-platdrv.c)
>
>> Thanks,
>> Luis
>>
>> On 10/18/2016 16:17, Wolfram Sang wrote:
>>>> This is needed because the configuration is different and the i2c-
>>>> designware
>>>> cannot be master/slave without a reset. To resolve that I added
>>>> this property
>>>> to bind it as a slave when needed.
>>> Aww, pity that the HW can't do that. Do you have details why?
>>>
>>> If that is really a HW limitation, then I'd suggest having a
>>> seperate
>>> driver for slave-only mode so we can differentiate by compatible
>>> strings.
>>>
>>
^ permalink raw reply
* [PATCH 0/3] Add basic support for the I2C units of the Armada 3700
From: Romain Perier @ 2016-11-08 15:15 UTC (permalink / raw)
To: Wolfram Sang, linux-i2c-u79uwXL29TY76Z2rM5mHXA
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Ian Campbell,
Pawel Moll, Mark Rutland, Kumar Gala, Jason Cooper, Andrew Lunn,
Sebastian Hesselbarth, Gregory Clement, Thomas Petazzoni,
Nadav Haklai, Omri Itach, Shadi Ammouri, Yahuda Yitschak,
Hanna Hawa, Neta Zur Hershkovits, Igal Liberman, Marcin Wojtas
This series add basic support for the I2C bus interface units present
in the Armada 3700 to the pxa-i2c driver. It also add the definitions of
the device nodes to the devicetree at the SoC level and for its official
development board: the Armada 3720 DB.
Romain Perier (3):
i2c: pxa: Add support for the I2C units found in Armada 3700
arm64: dts: marvell: Add I2C definitions for the Armada 3700
dt-bindings: i2c: pxa: Update the documentation for the Armada 3700
Documentation/devicetree/bindings/i2c/i2c-pxa.txt | 1 +
arch/arm64/boot/dts/marvell/armada-3720-db.dts | 4 ++++
arch/arm64/boot/dts/marvell/armada-37xx.dtsi | 18 ++++++++++++++++
drivers/i2c/busses/Kconfig | 2 +-
drivers/i2c/busses/i2c-pxa.c | 25 +++++++++++++++++++++--
5 files changed, 47 insertions(+), 3 deletions(-)
--
2.9.3
--
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
* [PATCH 1/3] i2c: pxa: Add support for the I2C units found in Armada 3700
From: Romain Perier @ 2016-11-08 15:15 UTC (permalink / raw)
To: Wolfram Sang, linux-i2c-u79uwXL29TY76Z2rM5mHXA
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Ian Campbell,
Pawel Moll, Mark Rutland, Kumar Gala, Jason Cooper, Andrew Lunn,
Sebastian Hesselbarth, Gregory Clement, Thomas Petazzoni,
Nadav Haklai, Omri Itach, Shadi Ammouri, Yahuda Yitschak,
Hanna Hawa, Neta Zur Hershkovits, Igal Liberman, Marcin Wojtas
In-Reply-To: <20161108151506.1131-1-romain.perier-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
The Armada 3700 has two I2C controllers that is compliant with the I2C
Bus Specificiation 2.1, supports multi-master and different bus speed:
Standard mode (up to 100 KHz), Fast mode (up to 400 KHz),
High speed mode (up to 3.4 Mhz).
This IP block has a lot of similarity with the PXA, except some register
offsets and bitfield. This commits adds a basic support for this I2C
unit.
Signed-off-by: Romain Perier <romain.perier-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---
drivers/i2c/busses/Kconfig | 2 +-
drivers/i2c/busses/i2c-pxa.c | 25 +++++++++++++++++++++++--
2 files changed, 24 insertions(+), 3 deletions(-)
diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index d252276..2f56a26 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -763,7 +763,7 @@ config I2C_PUV3
config I2C_PXA
tristate "Intel PXA2XX I2C adapter"
- depends on ARCH_PXA || ARCH_MMP || (X86_32 && PCI && OF)
+ depends on ARCH_PXA || ARCH_MMP || ARCH_MVEBU || (X86_32 && PCI && OF)
help
If you have devices in the PXA I2C bus, say yes to this option.
This driver can also be built as a module. If so, the module
diff --git a/drivers/i2c/busses/i2c-pxa.c b/drivers/i2c/busses/i2c-pxa.c
index e28b825..34ea830 100644
--- a/drivers/i2c/busses/i2c-pxa.c
+++ b/drivers/i2c/busses/i2c-pxa.c
@@ -55,6 +55,7 @@ enum pxa_i2c_types {
REGS_PXA3XX,
REGS_CE4100,
REGS_PXA910,
+ REGS_A3700,
};
/*
@@ -91,6 +92,13 @@ static struct pxa_reg_layout pxa_reg_layout[] = {
.ilcr = 0x28,
.iwcr = 0x30,
},
+ [REGS_A3700] = {
+ .ibmr = 0x00,
+ .idbr = 0x04,
+ .icr = 0x08,
+ .isr = 0x0c,
+ .isar = 0x10,
+ },
};
static const struct platform_device_id i2c_pxa_id_table[] = {
@@ -98,6 +106,7 @@ static const struct platform_device_id i2c_pxa_id_table[] = {
{ "pxa3xx-pwri2c", REGS_PXA3XX },
{ "ce4100-i2c", REGS_CE4100 },
{ "pxa910-i2c", REGS_PXA910 },
+ { "armada-3700-i2c", REGS_A3700 },
{ },
};
MODULE_DEVICE_TABLE(platform, i2c_pxa_id_table);
@@ -122,7 +131,9 @@ MODULE_DEVICE_TABLE(platform, i2c_pxa_id_table);
#define ICR_SADIE (1 << 13) /* slave address detected int enable */
#define ICR_UR (1 << 14) /* unit reset */
#define ICR_FM (1 << 15) /* fast mode */
+#define ICR_BUSMODE_FM (1 << 16) /* shifted fast mode for armada-3700 */
#define ICR_HS (1 << 16) /* High Speed mode */
+#define ICR_BUSMODE_HS (1 << 17) /* shifted high speed mode for armada-3700 */
#define ICR_GPIOEN (1 << 19) /* enable GPIO mode for SCL in HS */
#define ISR_RWM (1 << 0) /* read/write mode */
@@ -193,6 +204,8 @@ struct pxa_i2c {
unsigned char master_code;
unsigned long rate;
bool highmode_enter;
+ unsigned long fm_mask;
+ unsigned long hs_mask;
};
#define _IBMR(i2c) ((i2c)->reg_ibmr)
@@ -503,8 +516,8 @@ static void i2c_pxa_reset(struct pxa_i2c *i2c)
writel(i2c->slave_addr, _ISAR(i2c));
/* set control register values */
- writel(I2C_ICR_INIT | (i2c->fast_mode ? ICR_FM : 0), _ICR(i2c));
- writel(readl(_ICR(i2c)) | (i2c->high_mode ? ICR_HS : 0), _ICR(i2c));
+ writel(I2C_ICR_INIT | (i2c->fast_mode ? i2c->fm_mask : 0), _ICR(i2c));
+ writel(readl(_ICR(i2c)) | (i2c->high_mode ? i2c->hs_mask : 0), _ICR(i2c));
#ifdef CONFIG_I2C_PXA_SLAVE
dev_info(&i2c->adap.dev, "Enabling slave mode\n");
@@ -1137,6 +1150,7 @@ static const struct of_device_id i2c_pxa_dt_ids[] = {
{ .compatible = "mrvl,pxa-i2c", .data = (void *)REGS_PXA2XX },
{ .compatible = "mrvl,pwri2c", .data = (void *)REGS_PXA3XX },
{ .compatible = "mrvl,mmp-twsi", .data = (void *)REGS_PXA910 },
+ { .compatible = "marvell,armada-3700-i2c", .data = (void *)REGS_A3700 },
{}
};
MODULE_DEVICE_TABLE(of, i2c_pxa_dt_ids);
@@ -1158,6 +1172,13 @@ static int i2c_pxa_probe_dt(struct platform_device *pdev, struct pxa_i2c *i2c,
i2c->use_pio = 1;
if (of_get_property(np, "mrvl,i2c-fast-mode", NULL))
i2c->fast_mode = 1;
+ if (of_device_is_compatible(np, "marvell,armada-3700-i2c")) {
+ i2c->fm_mask = ICR_BUSMODE_FM;
+ i2c->hs_mask = ICR_BUSMODE_HS;
+ } else {
+ i2c->fm_mask = ICR_FM;
+ i2c->hs_mask = ICR_HS;
+ }
*i2c_types = (enum pxa_i2c_types)(of_id->data);
--
2.9.3
--
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 3/3] dt-bindings: i2c: pxa: Update the documentation for the Armada 3700
From: Romain Perier @ 2016-11-08 15:15 UTC (permalink / raw)
To: Wolfram Sang, linux-i2c
Cc: devicetree, Rob Herring, Ian Campbell, Pawel Moll, Mark Rutland,
Kumar Gala, Jason Cooper, Andrew Lunn, Sebastian Hesselbarth,
Gregory Clement, Thomas Petazzoni, Nadav Haklai, Omri Itach,
Shadi Ammouri, Yahuda Yitschak, Hanna Hawa, Neta Zur Hershkovits,
Igal Liberman, Marcin Wojtas
In-Reply-To: <20161108151506.1131-1-romain.perier@free-electrons.com>
This commit documents the compatible string to have the compatibility for
the I2C unit found in the Armada 3700.
Signed-off-by: Romain Perier <romain.perier@free-electrons.com>
---
Documentation/devicetree/bindings/i2c/i2c-pxa.txt | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/i2c/i2c-pxa.txt b/Documentation/devicetree/bindings/i2c/i2c-pxa.txt
index 12b78ac..b1b995c 100644
--- a/Documentation/devicetree/bindings/i2c/i2c-pxa.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c-pxa.txt
@@ -7,6 +7,7 @@ Required properties :
compatible processor, e.g. pxa168, pxa910, mmp2, mmp3.
For the pxa2xx/pxa3xx, an additional node "mrvl,pxa-i2c" is required
as shown in the example below.
+ For the Armada 3700, the compatible should be "marvell,armada-3700".
Recommended properties :
--
2.9.3
^ permalink raw reply related
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