Re: [PATCH 2/2] extcon: sm5502: EXTCON_SM5502 should depend on I2C
Dear Myungjoo, On 08/21/2014 02:27 PM, MyungJoo Ham wrote: >> Hi Geert >> >> Thanks for your report. I already sent a patch[1] to fix this build break >> and I'll send pull request to includec this patch in 3.17-rc2. >> >> [1] https://lkml.org/lkml/2014/8/13/761 >> >> Best Regards, >> Chanwoo Choi > > I do not object to this patch or your patch[1]. > > However, wouldn't it be better to add depends on I2C at REGMAP_I2C? > When you use REGMAP_I2C, you assume that I2C is already there, don't you? The previous REGMAP_I2C has not the dependency on I2C. So, Greert posted following patch[1] to fix it. [1] https://lkml.org/lkml/2014/8/17/27 Also, if I2C is 'm' (module) and some driver has not dependency on I2C, build break happen. Thanks Chanwoo Choi, > > > Cheers, > MyungJoo > >> >> >> On 08/17/2014 07:08 PM, Geert Uytterhoeven wrote: >>> EXTCON_SM5502 selects REGMAP_I2C, but if I2C=n: >>> >>> drivers/base/regmap/regmap-i2c.c: In function ‘regmap_smbus_byte_reg_read’: >>> drivers/base/regmap/regmap-i2c.c:28: error: implicit declaration of >>> function ‘i2c_smbus_read_byte_data’ >>> drivers/base/regmap/regmap-i2c.c: In function ‘regmap_smbus_byte_reg_write’: >>> drivers/base/regmap/regmap-i2c.c:46: error: implicit declaration of >>> function ‘i2c_smbus_write_byte_data’ >>> drivers/base/regmap/regmap-i2c.c: In function ‘regmap_smbus_word_reg_read’: >>> drivers/base/regmap/regmap-i2c.c:64: error: implicit declaration of >>> function ‘i2c_smbus_read_word_data’ >>> drivers/base/regmap/regmap-i2c.c: In function ‘regmap_smbus_word_reg_write’: >>> drivers/base/regmap/regmap-i2c.c:82: error: implicit declaration of >>> function ‘i2c_smbus_write_word_data’ >>> drivers/base/regmap/regmap-i2c.c: In function ‘regmap_i2c_write’: >>> drivers/base/regmap/regmap-i2c.c:96: error: implicit declaration of >>> function ‘i2c_master_send’ >>> drivers/base/regmap/regmap-i2c.c: In function ‘regmap_i2c_gather_write’: >>> drivers/base/regmap/regmap-i2c.c:117: error: implicit declaration of >>> function ‘i2c_check_functionality’ >>> drivers/base/regmap/regmap-i2c.c:130: error: implicit declaration of >>> function ‘i2c_transfer’ >>> >>> Signed-off-by: Geert Uytterhoeven >>> --- >>> drivers/extcon/Kconfig | 1 + >>> 1 file changed, 1 insertion(+) >>> >>> diff --git a/drivers/extcon/Kconfig b/drivers/extcon/Kconfig >>> index 6f2f4727de2c..764f3a113e0a 100644 >>> --- a/drivers/extcon/Kconfig >>> +++ b/drivers/extcon/Kconfig >>> @@ -72,6 +72,7 @@ config EXTCON_PALMAS >>> >>> config EXTCON_SM5502 >>> tristate "SM5502 EXTCON support" >>> + depends on I2C >>> select IRQ_DOMAIN >>> select REGMAP_I2C >>> select REGMAP_IRQ >>> >> >> >> >> >> >> >> >> -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Re: [PATCH 2/2] extcon: sm5502: EXTCON_SM5502 should depend on I2C
> Hi Geert
>
> Thanks for your report. I already sent a patch[1] to fix this build break
> and I'll send pull request to includec this patch in 3.17-rc2.
>
> [1] https://lkml.org/lkml/2014/8/13/761
>
> Best Regards,
> Chanwoo Choi
I do not object to this patch or your patch[1].
However, wouldn't it be better to add depends on I2C at REGMAP_I2C?
When you use REGMAP_I2C, you assume that I2C is already there, don't you?
Cheers,
MyungJoo
>
>
> On 08/17/2014 07:08 PM, Geert Uytterhoeven wrote:
> > EXTCON_SM5502 selects REGMAP_I2C, but if I2C=n:
> >
> > drivers/base/regmap/regmap-i2c.c: In function ‘regmap_smbus_byte_reg_read’:
> > drivers/base/regmap/regmap-i2c.c:28: error: implicit declaration of
> > function ‘i2c_smbus_read_byte_data’
> > drivers/base/regmap/regmap-i2c.c: In function ‘regmap_smbus_byte_reg_write’:
> > drivers/base/regmap/regmap-i2c.c:46: error: implicit declaration of
> > function ‘i2c_smbus_write_byte_data’
> > drivers/base/regmap/regmap-i2c.c: In function ‘regmap_smbus_word_reg_read’:
> > drivers/base/regmap/regmap-i2c.c:64: error: implicit declaration of
> > function ‘i2c_smbus_read_word_data’
> > drivers/base/regmap/regmap-i2c.c: In function ‘regmap_smbus_word_reg_write’:
> > drivers/base/regmap/regmap-i2c.c:82: error: implicit declaration of
> > function ‘i2c_smbus_write_word_data’
> > drivers/base/regmap/regmap-i2c.c: In function ‘regmap_i2c_write’:
> > drivers/base/regmap/regmap-i2c.c:96: error: implicit declaration of
> > function ‘i2c_master_send’
> > drivers/base/regmap/regmap-i2c.c: In function ‘regmap_i2c_gather_write’:
> > drivers/base/regmap/regmap-i2c.c:117: error: implicit declaration of
> > function ‘i2c_check_functionality’
> > drivers/base/regmap/regmap-i2c.c:130: error: implicit declaration of
> > function ‘i2c_transfer’
> >
> > Signed-off-by: Geert Uytterhoeven
> > ---
> > drivers/extcon/Kconfig | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/extcon/Kconfig b/drivers/extcon/Kconfig
> > index 6f2f4727de2c..764f3a113e0a 100644
> > --- a/drivers/extcon/Kconfig
> > +++ b/drivers/extcon/Kconfig
> > @@ -72,6 +72,7 @@ config EXTCON_PALMAS
> >
> > config EXTCON_SM5502
> > tristate "SM5502 EXTCON support"
> > + depends on I2C
> > select IRQ_DOMAIN
> > select REGMAP_I2C
> > select REGMAP_IRQ
> >
>
>
>
>
>
>
>
>
N떑꿩�r툤y鉉싕b쾊Ф푤v�^�)頻{.n�+돴쪐{콏g"왲^n뇊⊆쫧�곷h솳鈺�&�� 췍쳺�h�(��쉸듶줷"얎�m��곴�z받뻿筬f＂톒쉱�~늤
[PULL REQUEST] i2c for 3.17
Linus, here is the fixup for the 'lowlight' of my last pull request. I2C is not selected anymore by I2C_ACPI. Instead, the code in question now depends on I2C=y. Also, Mika has agreed to support me and be the maintainer for I2C-ACPI related patches. Finally, a new-ID-patch came along last week. Please pull, Wolfram The following changes since commit 7d1311b93e58ed55f3a31cc8f94c4b8fe988a2b9: Linux 3.17-rc1 (2014-08-16 10:40:26 -0600) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git i2c/for-next for you to fetch changes up to 4560d67722816cca4b2f3dfb1d7c5b902fd2075b: MAINTAINERS: add maintainer for ACPI parts of I2C (2014-08-19 10:34:08 -0500) Alan Cox (1): i2c: i801: Add PCI ID for Intel Braswell Lan Tianyu (1): i2c: rework kernel config I2C_ACPI Wolfram Sang (1): MAINTAINERS: add maintainer for ACPI parts of I2C MAINTAINERS | 7 +++ drivers/i2c/Kconfig | 15 ++- drivers/i2c/Makefile | 2 +- drivers/i2c/busses/i2c-i801.c | 2 ++ drivers/i2c/i2c-acpi.c| 2 ++ include/linux/i2c.h | 12 6 files changed, 26 insertions(+), 14 deletions(-) signature.asc Description: Digital signature
Re: [PATCH 2/2] extcon: sm5502: EXTCON_SM5502 should depend on I2C
Hi Geert Thanks for your report. I already sent a patch[1] to fix this build break and I'll send pull request to includec this patch in 3.17-rc2. [1] https://lkml.org/lkml/2014/8/13/761 Best Regards, Chanwoo Choi On 08/17/2014 07:08 PM, Geert Uytterhoeven wrote: > EXTCON_SM5502 selects REGMAP_I2C, but if I2C=n: > > drivers/base/regmap/regmap-i2c.c: In function ‘regmap_smbus_byte_reg_read’: > drivers/base/regmap/regmap-i2c.c:28: error: implicit declaration of function > ‘i2c_smbus_read_byte_data’ > drivers/base/regmap/regmap-i2c.c: In function ‘regmap_smbus_byte_reg_write’: > drivers/base/regmap/regmap-i2c.c:46: error: implicit declaration of function > ‘i2c_smbus_write_byte_data’ > drivers/base/regmap/regmap-i2c.c: In function ‘regmap_smbus_word_reg_read’: > drivers/base/regmap/regmap-i2c.c:64: error: implicit declaration of function > ‘i2c_smbus_read_word_data’ > drivers/base/regmap/regmap-i2c.c: In function ‘regmap_smbus_word_reg_write’: > drivers/base/regmap/regmap-i2c.c:82: error: implicit declaration of function > ‘i2c_smbus_write_word_data’ > drivers/base/regmap/regmap-i2c.c: In function ‘regmap_i2c_write’: > drivers/base/regmap/regmap-i2c.c:96: error: implicit declaration of function > ‘i2c_master_send’ > drivers/base/regmap/regmap-i2c.c: In function ‘regmap_i2c_gather_write’: > drivers/base/regmap/regmap-i2c.c:117: error: implicit declaration of function > ‘i2c_check_functionality’ > drivers/base/regmap/regmap-i2c.c:130: error: implicit declaration of function > ‘i2c_transfer’ > > Signed-off-by: Geert Uytterhoeven > --- > drivers/extcon/Kconfig | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/extcon/Kconfig b/drivers/extcon/Kconfig > index 6f2f4727de2c..764f3a113e0a 100644 > --- a/drivers/extcon/Kconfig > +++ b/drivers/extcon/Kconfig > @@ -72,6 +72,7 @@ config EXTCON_PALMAS > > config EXTCON_SM5502 > tristate "SM5502 EXTCON support" > + depends on I2C > select IRQ_DOMAIN > select REGMAP_I2C > select REGMAP_IRQ > -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] i2c:at91: add bound checking on SMBus block length bytes
The driver was not bound checking the received length byte to ensure it was
within the
the buffer size that is allocated for SMBus blocks. This resulted in buffer
overflows
whenever an invalid length byte was received.
It also failed to ensure the length byte was not zero. If it received zero, it
would end up
in an infinite loop as the at91_twi_read_next_byte function returned
immediately without
allowing RHR to be read to clear the RXRDY interrupt.
Tested agaisnt a SMBus compliant battery.
Signed-off-by: Marek Roszko
Acked-by: Ludovic Desroches
---
Change from v1:
fixed typo in commit message
reworded message slightly to be specifically say length byte
drivers/i2c/busses/i2c-at91.c | 28
1 file changed, 24 insertions(+), 4 deletions(-)
diff --git a/drivers/i2c/busses/i2c-at91.c b/drivers/i2c/busses/i2c-at91.c
index e95f9ba..ec4ff33 100644
--- a/drivers/i2c/busses/i2c-at91.c
+++ b/drivers/i2c/busses/i2c-at91.c
@@ -101,6 +101,7 @@ struct at91_twi_dev {
unsigned twi_cwgr_reg;
struct at91_twi_pdata *pdata;
bool use_dma;
+ bool recv_len_abort;
struct at91_twi_dma dma;
};
@@ -267,12 +268,24 @@ static void at91_twi_read_next_byte(struct at91_twi_dev
*dev)
*dev->buf = at91_twi_read(dev, AT91_TWI_RHR) & 0xff;
--dev->buf_len;
+ /* return if aborting, we only needed to read RHR to clear RXRDY*/
+ if (dev->recv_len_abort)
+ return;
+
/* handle I2C_SMBUS_BLOCK_DATA */
if (unlikely(dev->msg->flags & I2C_M_RECV_LEN)) {
- dev->msg->flags &= ~I2C_M_RECV_LEN;
- dev->buf_len += *dev->buf;
- dev->msg->len = dev->buf_len + 1;
- dev_dbg(dev->dev, "received block length %d\n", dev->buf_len);
+ /* ensure length byte is a valid value */
+ if (*dev->buf <= I2C_SMBUS_BLOCK_MAX && *dev->buf > 0) {
+ dev->msg->flags &= ~I2C_M_RECV_LEN;
+ dev->buf_len += *dev->buf;
+ dev->msg->len = dev->buf_len + 1;
+ dev_dbg(dev->dev, "received block length %d\n",
+dev->buf_len);
+ } else {
+ /* abort and send the stop by reading one more byte */
+ dev->recv_len_abort = true;
+ dev->buf_len = 1;
+ }
}
/* send stop if second but last byte has been read */
@@ -444,6 +457,12 @@ static int at91_do_twi_transfer(struct at91_twi_dev *dev)
ret = -EIO;
goto error;
}
+ if (dev->recv_len_abort) {
+ dev_err(dev->dev, "invalid smbus block length recvd\n");
+ ret = -EPROTO;
+ goto error;
+ }
+
dev_dbg(dev->dev, "transfer complete\n");
return 0;
@@ -500,6 +519,7 @@ static int at91_twi_xfer(struct i2c_adapter *adap, struct
i2c_msg *msg, int num)
dev->buf_len = m_start->len;
dev->buf = m_start->buf;
dev->msg = m_start;
+ dev->recv_len_abort = false;
ret = at91_do_twi_transfer(dev);
--
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RESEND PATCH 4/7] mfd: cros_ec: move locking into cros_ec_cmd_xfer
Javier, On Wed, Aug 20, 2014 at 5:13 AM, Javier Martinez Canillas wrote: > From: Andrew Bresticker > > Now that there's a central cros_ec_cmd_xfer(), move the locking > out of the SPI and LPC drivers. Slight nit that the LPC driver doesn't exist upstream. This is in prep for adding the LPC driver, though. > Signed-off-by: Andrew Bresticker > Reviewed-by: Simon Glass > Signed-off-by: Javier Martinez Canillas > --- > drivers/mfd/cros_ec.c | 10 +- > drivers/mfd/cros_ec_spi.c | 11 --- > 2 files changed, 9 insertions(+), 12 deletions(-) After comment nitfix: Reviewed-by: Doug Anderson -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RESEND PATCH 3/7] mfd: cros_ec: stop calling ->cmd_xfer() directly
Javier, On Wed, Aug 20, 2014 at 5:13 AM, Javier Martinez Canillas wrote: > From: Andrew Bresticker > > Instead of having users of the ChromeOS EC call the interface-specific > cmd_xfer() callback directly, introduce a central cros_ec_cmd_xfer() > to use instead. This will allow us to put all the locking and retry > logic in one place instead of duplicating it across the different > drivers. > > Signed-off-by: Andrew Bresticker > Reviewed-by: Simon Glass > Signed-off-by: Javier Martinez Canillas > --- > drivers/i2c/busses/i2c-cros-ec-tunnel.c | 2 +- > drivers/input/keyboard/cros_ec_keyb.c | 2 +- > drivers/mfd/cros_ec.c | 7 +++ > include/linux/mfd/cros_ec.h | 24 ++-- > 4 files changed, 27 insertions(+), 8 deletions(-) Reviewed-by: Doug Anderson -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] usb: add support for Diolan DLN-2 devices
On Wednesday 20 August 2014, Daniel Baluta wrote: > From: Octavian Purdila > > This patch implements the USB part of the Diolan USB-I2C/SPI/GPIO > Master Adapter DLN-2. Details about the device can be found here: > > https://www.diolan.com/i2c/i2c_interface.html. > > Information about the USB protocol can be found in the Programmer's > Reference Manual [1], see section 1.7. > > Because the hardware has a single transmit endpoint and a single > receive endpoint the communication between the various DLN2 drivers > and the hardware will be muxed/demuxed by this driver. > > The functional DLN2 drivers (i2c, GPIO, etc.) will have to register > themselves as DLN2 modules in order to send or receive data. > > Each DLN2 module will be identified by the handle field within the DLN2 > message header. If a DLN2 module issues multiple commands in parallel > they will be identified by the echo counter field in the message header. > > The DLN2 modules can use the dln2_transfer() function to issue a > command and wait for its response. They can also use an asynchronous > mode of operation, in which case a receive callback function is going > to be notified when messages for a specific handle are received. > > Because the hardware reserves handle 0 for GPIO events, the driver > also reserves handle 0. It will be allocated to a DLN2 module only if > it is explicitly requested. > > [1] https://www.diolan.com/downloads/dln-api-manual.pdf > > Signed-off-by: Octavian Purdila After a very brief review of the driver, I think this would be better handled as an MFD driver in drivers/mfd that creates child devices and has the high-level drivers get registered as platform_driver. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC] i2c-smbus: smbus alert revisited
This follows on from the discussion in late March before Srinivas got
busy on other things.
This is just an RFC for a next generation set of patches. The idea is as
follows
- If an adapter knows about its ARA and smbus alerts then the adapter
creates its own interrupt handler as before
- If a client knows it needs smbus alerts it calls
i2c_require_smbus_alert. This ensures that there is an ARA handler
registered and tells the client whether the adapter is handling it
anyway or not.
- When the client learns that an ARA event has occurred it calls
i2c_smbus_alert_event which uses the existing ARA mechanism to kick
things off.
Leaving it to the client and introducing that little bit of asymmetry
means
- we don't have to do expensive polling
- we cope with the case where there isn't a single smbalert line instead
each device has a GPIO
- we cope with the case of devices that need to trigger an ARA in other
ways (eg with no IRQ or GPIO but discovering a particular bit is set
and needing an ARA to unwedge it)
It also gives us a hook from the client so that in future we can
address the case of adapters that can do SMB alert in hardware but are
sometimes wired to devices that use the same port and are not using ARA.
This is just an RFC, it compiles but thats all I can say for sure. Just
want to know if this is the right direction before actually doing any
more serious work on it.
Alan
---
diff --git a/drivers/i2c/i2c-smbus.c b/drivers/i2c/i2c-smbus.c
index fc99f0d..955352c 100644
--- a/drivers/i2c/i2c-smbus.c
+++ b/drivers/i2c/i2c-smbus.c
@@ -30,7 +30,7 @@
#include
struct i2c_smbus_alert {
- unsigned intalert_edge_triggered:1;
+ unsigned intalert_edge_triggered:1, client:1;
int irq;
struct work_struct alert;
struct i2c_client *ara; /* Alert response address */
@@ -123,6 +123,29 @@ static void smbus_alert(struct work_struct *work)
enable_irq(alert->irq);
}
+/**
+ * i2c_smbus_alert_event - possible ARA event
+ * @client: client which thinks an ARA may have occurred
+ *
+ * Called by i2c clients that have requested ARA support and been
+ * advised there is no host adapter interrupt. This may be invoked
+ * from a driver specific IRQ, from driver polling or when the
+ * driver discovers by other means that an ARA may be present
+ */
+void i2c_smbus_alert_event(struct i2c_client *client)
+{
+ struct i2c_adapter *adapter = client->adapter;
+ struct i2c_client *smbus_ara = adapter->smbus_ara;
+ struct i2c_smbus_alert *alert;
+
+ if (smbus_ara != NULL) {
+ alert = i2c_get_clientdata(smbus_ara);
+ if (alert->client)
+ schedule_work(&alert->alert);
+ }
+}
+EXPORT_SYMBOL_GPL(i2c_smbus_alert_event);
+
static irqreturn_t smbalert_irq(int irq, void *d)
{
struct i2c_smbus_alert *alert = d;
@@ -151,6 +174,7 @@ static int smbalert_probe(struct i2c_client *ara,
alert->alert_edge_triggered = setup->alert_edge_triggered;
alert->irq = setup->irq;
+ alert->client = setup->client;
INIT_WORK(&alert->alert, smbus_alert);
alert->ara = ara;
@@ -162,8 +186,9 @@ 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");
+ if (setup->client == 0)
+ dev_info(&adapter->dev, "supports SMBALERT#, %s trigger\n",
+setup->alert_edge_triggered ? "edge" : "level");
return 0;
}
@@ -201,7 +226,9 @@ static struct i2c_driver smbalert_driver = {
* Setup handling of the SMBus alert protocol on a given I2C bus segment.
*
* Handling can be done either through our IRQ handler, or by the
- * adapter (from its handler, periodic polling, or whatever).
+ * adapter (from its handler, periodic polling, or whatever), or can
+ * be configured by a client for cases where we only learn the IRQ from
+ * later discovery.
*
* NOTE that if we manage the IRQ, we *MUST* know if it's level or
* edge triggered in order to hand it to the workqueue correctly.
@@ -219,8 +246,11 @@ struct i2c_client *i2c_setup_smbus_alert(struct
i2c_adapter *adapter,
I2C_BOARD_INFO("smbus_alert", 0x0c),
.platform_data = setup,
};
+ struct i2c_client *ret;
- return i2c_new_device(adapter, &ara_board_info);
+ ret = i2c_new_device(adapter, &ara_board_info);
+ adapter->smbus_ara = ret;
+ return ret;
}
EXPORT_SYMBOL_GPL(i2c_setup_smbus_alert);
@@ -246,6 +276,54 @@ EXPORT_SYMBOL_GPL(i2c_handle_smbus_alert);
module_i2c_driver(smbalert_driver);
+/**
+ * i2c_require_smbus_alert - Client discovered SMBus alert
+ * @c: client requiring ARA
+ *
+ * When a client needs an ARA
Re: [PATCH v3 0/7] i2c: Relax mandatory I2C ID table passing
On Thu, 17 Jul 2014, Wolfram Sang wrote: > On Thu, Jul 17, 2014 at 11:23:42AM +0100, Lee Jones wrote: > > Hi Wolfram, > > > > Are you going to take a look at this set? > > Sure thing, yet I won't make it for 3.17, sadly :( It has high priority > for 3.18, though. Do you want me to re-send now the -rc1 had been released? -- Lee Jones Linaro STMicroelectronics Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] i2c designware add support of I2C standard mode
On Wed, 20 Aug 2014, atull wrote:
>
>
> On Wed, 20 Aug 2014, Romain Baeriswyl wrote:
>
> > From: Romain Baeriswyl
> >
> > Some legacy devices support ony I2C standard mode at 100kHz.
> > This patch allows to select the standard mode through the DTS
> > with the use of the existing clock-frequency parameter.
> >
> > When clock-frequency parameter is not set, the fast mode is selected.
> > Only when the parameter is set at 10, the standard mode is selected.
> >
> > Signed-off-by: Romain Baeriswyl
> > Reviewed-by: Christian Ruppert
> > ---
> > drivers/i2c/busses/i2c-designware-platdrv.c | 23 +--
> > 1 files changed, 21 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c
> > b/drivers/i2c/busses/i2c-designware-platdrv.c
> > index 402ec39..b543fe1 100644
> > --- a/drivers/i2c/busses/i2c-designware-platdrv.c
> > +++ b/drivers/i2c/busses/i2c-designware-platdrv.c
> > @@ -122,6 +122,7 @@ static int dw_i2c_probe(struct platform_device *pdev)
> > struct i2c_adapter *adap;
> > struct resource *mem;
> > int irq, r;
> > + u32 clk_freq;
> >
> > irq = platform_get_irq(pdev, 0);
> > if (irq < 0) {
> > @@ -151,6 +152,9 @@ static int dw_i2c_probe(struct platform_device *pdev)
> > return PTR_ERR(dev->clk);
> > clk_prepare_enable(dev->clk);
> >
> > + /* fast mode by default because of legacy reasons */
> > + clk_freq = 40;
> > +
> > if (pdev->dev.of_node) {
> > u32 ht = 0;
> > u32 ic_clk = dev->get_clk_rate_khz(dev);
> > @@ -166,6 +170,17 @@ static int dw_i2c_probe(struct platform_device *pdev)
> > of_property_read_u32(pdev->dev.of_node,
> > "i2c-scl-falling-time-ns",
> > &dev->scl_falling_time);
> > +
> > + of_property_read_u32(pdev->dev.of_node, "clock-frequency",
> > +&clk_freq);
> > +
> > + /* Only standard mode at 100kHz and fast mode at 400kHz
> > +* are supported.
> > +*/
> > + if (clk_freq != 10 && clk_freq != 40) {
> > + dev_err(&pdev->dev, "Only 100kHz and 400kHz supported");
> > + return -EINVAL;
> > + }
>
> Hi Romain,
>
> It is common to operate i2c at <100KHz on boards that have i2c
> issues. I am using this driver at 50KHz because I have a LCD module
> that just doesn't work at a full 100KHz. Please remove the check for
> these two frequency points.
>
> > }
> >
> > dev->functionality =
> > @@ -175,8 +190,12 @@ static int dw_i2c_probe(struct platform_device *pdev)
> > I2C_FUNC_SMBUS_BYTE_DATA |
> > I2C_FUNC_SMBUS_WORD_DATA |
> > I2C_FUNC_SMBUS_I2C_BLOCK;
> > - dev->master_cfg = DW_IC_CON_MASTER | DW_IC_CON_SLAVE_DISABLE |
> > - DW_IC_CON_RESTART_EN | DW_IC_CON_SPEED_FAST;
> > + if (clk_freq == 10)
>
> Please change to <=
>
Hi Romain,
This looks fine to me, my "<= issues" are not important as this isn't
really setting a frequency, but just setting a bit that controls speed.
Acked-by: Alan Tull
> > + dev->master_cfg = DW_IC_CON_MASTER | DW_IC_CON_SLAVE_DISABLE |
> > + DW_IC_CON_RESTART_EN | DW_IC_CON_SPEED_STD;
> > + else
> > + dev->master_cfg = DW_IC_CON_MASTER | DW_IC_CON_SLAVE_DISABLE |
> > + DW_IC_CON_RESTART_EN | DW_IC_CON_SPEED_FAST;
> >
> > /* Try first if we can configure the device from ACPI */
> > r = dw_i2c_acpi_configure(pdev);
> > --
> > 1.7.1
> >
> >
>
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] i2c: designware: deduce speed mode from device tree setting
On Wed, 20 Aug 2014, Romain Baeriswyl wrote:
> Hi Alan,
>
> We got board issue using I2C and they were solved by changing the i2c timing.
>
> It is possible to change the tLOW and tHIGH period with the following
> parameters:
> i2c-sda-hold-time-ns
> i2c-sda-falling-time-ns
> i2c-scl-falling-time-ns
>
> Romain
Hi Romain,
This thread is getting kind of messed up. Yep, that's how I am slowing
down the bus, using i2c-sda-falling-time-ns and i2c-scl-falling-time-ns.
It is fine with me if you want to test for == 100KHz, just not my
(probably hair-splitting) preference. :)
Alan
>
> - Original Message -
> From: "atull"
> To: "Romain Baeriswyl"
> Cc: "Mark Rutland" , w...@the-dreams.de,
> bar...@tkos.co.il, "mika westerberg" ,
> "grant likely" , robh...@kernel.org,
> skuri...@pobox.com, "rafael j wysocki" ,
> a...@linux.intel.com, linux-i2c@vger.kernel.org,
> linux-ker...@vger.kernel.org, devicet...@vger.kernel.org, "delicious quinoa"
> , dingu...@opensource.altera.com,
> yvand...@opensource.altera.com
> Sent: Wednesday, August 20, 2014 4:24:45 PM
> Subject: Re: [PATCH] i2c: designware: deduce speed mode from device tree
> setting
>
>
>
> On Wed, 20 Aug 2014, Romain Baeriswyl wrote:
>
> > Hi,
> >
> > With the patch "i2c designware add support of I2C standard mode" I already
> > proposed:
> > - I2C standard mode is selected with 100kHz clock frequency.
> > - I2C fast mode is selected with 400kHy clock frequency.
> > - EINVAL error is returned if clock frequency is not 10 and not 40.
> >
> > but this patch seems not available yet.
> > What about the other patch "i2c designware make SCL and SDA falling time
> > configurable" ?
> >
> > In i2c-designware_platdrv.c the flag DW_IC_CON_SPEED_xxx is well set
> > depending on the mode:
> >
> > if (clk_freq == 10)
>
> Romain,
>
> I'm really happy if your patches get accepted. Can this be <= 10?
> It is really common to run I2C at a lower speed if you have some
> board issues with the i2c bus.
>
> Alan
>
> > dev->master_cfg = DW_IC_CON_MASTER | DW_IC_CON_SLAVE_DISABLE |
> >DW_IC_CON_RESTART_EN | DW_IC_CON_SPEED_STD;
> >else
> > dev->master_cfg = DW_IC_CON_MASTER | DW_IC_CON_SLAVE_DISABLE |
> >DW_IC_CON_RESTART_EN | DW_IC_CON_SPEED_FAST;
> >
> >
> > So for me everything should be fine if there patches are applied.
> >
> > Regards,
> >
> > Romain
> >
> > - Original Message -
> > From: "Mark Rutland"
> > To: at...@opensource.altera.com
> > Cc: w...@the-dreams.de, bar...@tkos.co.il, "mika westerberg"
> > , "grant likely"
> > , robh...@kernel.org, skuri...@pobox.com, "Romain
> > Baeriswyl" , "rafael j wysocki"
> > , a...@linux.intel.com,
> > linux-i2c@vger.kernel.org, linux-ker...@vger.kernel.org,
> > devicet...@vger.kernel.org, "delicious quinoa"
> > , dingu...@opensource.altera.com,
> > yvand...@opensource.altera.com
> > Sent: Wednesday, August 20, 2014 11:22:57 AM
> > Subject: Re: [PATCH] i2c: designware: deduce speed mode from device tree
> > setting
> >
> > On Tue, Aug 19, 2014 at 09:18:49PM +0100, at...@opensource.altera.com wrote:
> > > From: Alan Tull
> > >
> > > Use the documented, but unimplemented "clock-frequency"
> > > Device Tree setting as a guide on whether to set the speed
> > > mode bits in DW_IC_CON to standard or fast i2c mode.
> > >
> > > Previously, the driver was hardwired to fast mode. Default
> > > to fast mode if the "clock-frequency" property is not present
> > > for backwards compatiblity.
> > >
> > > Signed-off-by: Alan Tull
> > > ---
> > > drivers/i2c/busses/i2c-designware-platdrv.c | 10 --
> > > 1 file changed, 8 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c
> > > b/drivers/i2c/busses/i2c-designware-platdrv.c
> > > index bc87733..18cd3d9 100644
> > > --- a/drivers/i2c/busses/i2c-designware-platdrv.c
> > > +++ b/drivers/i2c/busses/i2c-designware-platdrv.c
> > > @@ -122,7 +122,8 @@ static int dw_i2c_probe(struct platform_device *pdev)
> > > struct dw_i2c_dev *dev;
> > > struct i2c_adapter *adap;
> > > struct resource *mem;
> > > - int irq, r;
> > > + int irq, r, ret, speed = DW_IC_CON_SPEED_FAST;
> > > + u32 bus_rate;
> > >
> > > irq = platform_get_irq(pdev, 0);
> > > if (irq < 0) {
> > > @@ -167,6 +168,11 @@ static int dw_i2c_probe(struct platform_device *pdev)
> > > of_property_read_u32(pdev->dev.of_node,
> > >"i2c-scl-falling-time-ns",
> > >&dev->scl_falling_time);
> > > +
> > > + ret = of_property_read_u32(pdev->dev.of_node,
> > > +"clock-frequency", &bus_rate);
> > > + if (!ret && (bus_rate <= 10))
> > > + speed = DW_IC_CON_SPEED_STD;
> >
> > This looks a bit odd.
> >
> > If the device only supports two particular speeds why do we accept any
> > other speed in the clock-frequency property? Sur
Re: [PATCH] i2c: designware: deduce speed mode from device tree setting
Hi Alan,
We got board issue using I2C and they were solved by changing the i2c timing.
It is possible to change the tLOW and tHIGH period with the following
parameters:
i2c-sda-hold-time-ns
i2c-sda-falling-time-ns
i2c-scl-falling-time-ns
Romain
- Original Message -
From: "atull"
To: "Romain Baeriswyl"
Cc: "Mark Rutland" , w...@the-dreams.de,
bar...@tkos.co.il, "mika westerberg" , "grant
likely" , robh...@kernel.org, skuri...@pobox.com,
"rafael j wysocki" , a...@linux.intel.com,
linux-i2c@vger.kernel.org, linux-ker...@vger.kernel.org,
devicet...@vger.kernel.org, "delicious quinoa" ,
dingu...@opensource.altera.com, yvand...@opensource.altera.com
Sent: Wednesday, August 20, 2014 4:24:45 PM
Subject: Re: [PATCH] i2c: designware: deduce speed mode from device tree setting
On Wed, 20 Aug 2014, Romain Baeriswyl wrote:
> Hi,
>
> With the patch "i2c designware add support of I2C standard mode" I already
> proposed:
> - I2C standard mode is selected with 100kHz clock frequency.
> - I2C fast mode is selected with 400kHy clock frequency.
> - EINVAL error is returned if clock frequency is not 10 and not 40.
>
> but this patch seems not available yet.
> What about the other patch "i2c designware make SCL and SDA falling time
> configurable" ?
>
> In i2c-designware_platdrv.c the flag DW_IC_CON_SPEED_xxx is well set
> depending on the mode:
>
> if (clk_freq == 10)
Romain,
I'm really happy if your patches get accepted. Can this be <= 10?
It is really common to run I2C at a lower speed if you have some
board issues with the i2c bus.
Alan
> dev->master_cfg = DW_IC_CON_MASTER | DW_IC_CON_SLAVE_DISABLE |
>DW_IC_CON_RESTART_EN | DW_IC_CON_SPEED_STD;
>else
> dev->master_cfg = DW_IC_CON_MASTER | DW_IC_CON_SLAVE_DISABLE |
>DW_IC_CON_RESTART_EN | DW_IC_CON_SPEED_FAST;
>
>
> So for me everything should be fine if there patches are applied.
>
> Regards,
>
> Romain
>
> - Original Message -
> From: "Mark Rutland"
> To: at...@opensource.altera.com
> Cc: w...@the-dreams.de, bar...@tkos.co.il, "mika westerberg"
> , "grant likely" ,
> robh...@kernel.org, skuri...@pobox.com, "Romain Baeriswyl"
> , "rafael j wysocki"
> , a...@linux.intel.com,
> linux-i2c@vger.kernel.org, linux-ker...@vger.kernel.org,
> devicet...@vger.kernel.org, "delicious quinoa" ,
> dingu...@opensource.altera.com, yvand...@opensource.altera.com
> Sent: Wednesday, August 20, 2014 11:22:57 AM
> Subject: Re: [PATCH] i2c: designware: deduce speed mode from device tree
> setting
>
> On Tue, Aug 19, 2014 at 09:18:49PM +0100, at...@opensource.altera.com wrote:
> > From: Alan Tull
> >
> > Use the documented, but unimplemented "clock-frequency"
> > Device Tree setting as a guide on whether to set the speed
> > mode bits in DW_IC_CON to standard or fast i2c mode.
> >
> > Previously, the driver was hardwired to fast mode. Default
> > to fast mode if the "clock-frequency" property is not present
> > for backwards compatiblity.
> >
> > Signed-off-by: Alan Tull
> > ---
> > drivers/i2c/busses/i2c-designware-platdrv.c | 10 --
> > 1 file changed, 8 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c
> > b/drivers/i2c/busses/i2c-designware-platdrv.c
> > index bc87733..18cd3d9 100644
> > --- a/drivers/i2c/busses/i2c-designware-platdrv.c
> > +++ b/drivers/i2c/busses/i2c-designware-platdrv.c
> > @@ -122,7 +122,8 @@ static int dw_i2c_probe(struct platform_device *pdev)
> > struct dw_i2c_dev *dev;
> > struct i2c_adapter *adap;
> > struct resource *mem;
> > - int irq, r;
> > + int irq, r, ret, speed = DW_IC_CON_SPEED_FAST;
> > + u32 bus_rate;
> >
> > irq = platform_get_irq(pdev, 0);
> > if (irq < 0) {
> > @@ -167,6 +168,11 @@ static int dw_i2c_probe(struct platform_device *pdev)
> > of_property_read_u32(pdev->dev.of_node,
> > "i2c-scl-falling-time-ns",
> > &dev->scl_falling_time);
> > +
> > + ret = of_property_read_u32(pdev->dev.of_node,
> > + "clock-frequency", &bus_rate);
> > + if (!ret && (bus_rate <= 10))
> > + speed = DW_IC_CON_SPEED_STD;
>
> This looks a bit odd.
>
> If the device only supports two particular speeds why do we accept any
> other speed in the clock-frequency property? Surely we should at least
> warn that something was off?
>
> Thanks,
> Mark
>
> > }
> >
> > dev->functionality =
> > @@ -177,7 +183,7 @@ static int dw_i2c_probe(struct platform_device *pdev)
> > I2C_FUNC_SMBUS_WORD_DATA |
> > I2C_FUNC_SMBUS_I2C_BLOCK;
> > dev->master_cfg = DW_IC_CON_MASTER | DW_IC_CON_SLAVE_DISABLE |
> > - DW_IC_CON_RESTART_EN | DW_IC_CON_SPEED_FAST;
> > + DW_IC_CON_RESTART_EN | speed;
> >
> > /* Try first if we can configure the device from ACPI */
> > r = dw_i2c_acp
Re: [PATCH] i2c designware add support of I2C standard mode
On Wed, 20 Aug 2014, Romain Baeriswyl wrote:
> From: Romain Baeriswyl
>
> Some legacy devices support ony I2C standard mode at 100kHz.
> This patch allows to select the standard mode through the DTS
> with the use of the existing clock-frequency parameter.
>
> When clock-frequency parameter is not set, the fast mode is selected.
> Only when the parameter is set at 10, the standard mode is selected.
>
> Signed-off-by: Romain Baeriswyl
> Reviewed-by: Christian Ruppert
> ---
> drivers/i2c/busses/i2c-designware-platdrv.c | 23 +--
> 1 files changed, 21 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c
> b/drivers/i2c/busses/i2c-designware-platdrv.c
> index 402ec39..b543fe1 100644
> --- a/drivers/i2c/busses/i2c-designware-platdrv.c
> +++ b/drivers/i2c/busses/i2c-designware-platdrv.c
> @@ -122,6 +122,7 @@ static int dw_i2c_probe(struct platform_device *pdev)
> struct i2c_adapter *adap;
> struct resource *mem;
> int irq, r;
> + u32 clk_freq;
>
> irq = platform_get_irq(pdev, 0);
> if (irq < 0) {
> @@ -151,6 +152,9 @@ static int dw_i2c_probe(struct platform_device *pdev)
> return PTR_ERR(dev->clk);
> clk_prepare_enable(dev->clk);
>
> + /* fast mode by default because of legacy reasons */
> + clk_freq = 40;
> +
> if (pdev->dev.of_node) {
> u32 ht = 0;
> u32 ic_clk = dev->get_clk_rate_khz(dev);
> @@ -166,6 +170,17 @@ static int dw_i2c_probe(struct platform_device *pdev)
> of_property_read_u32(pdev->dev.of_node,
>"i2c-scl-falling-time-ns",
>&dev->scl_falling_time);
> +
> + of_property_read_u32(pdev->dev.of_node, "clock-frequency",
> + &clk_freq);
> +
> + /* Only standard mode at 100kHz and fast mode at 400kHz
> + * are supported.
> + */
> + if (clk_freq != 10 && clk_freq != 40) {
> + dev_err(&pdev->dev, "Only 100kHz and 400kHz supported");
> + return -EINVAL;
> + }
Hi Romain,
It is common to operate i2c at <100KHz on boards that have i2c
issues. I am using this driver at 50KHz because I have a LCD module
that just doesn't work at a full 100KHz. Please remove the check for
these two frequency points.
> }
>
> dev->functionality =
> @@ -175,8 +190,12 @@ static int dw_i2c_probe(struct platform_device *pdev)
> I2C_FUNC_SMBUS_BYTE_DATA |
> I2C_FUNC_SMBUS_WORD_DATA |
> I2C_FUNC_SMBUS_I2C_BLOCK;
> - dev->master_cfg = DW_IC_CON_MASTER | DW_IC_CON_SLAVE_DISABLE |
> - DW_IC_CON_RESTART_EN | DW_IC_CON_SPEED_FAST;
> + if (clk_freq == 10)
Please change to <=
> + dev->master_cfg = DW_IC_CON_MASTER | DW_IC_CON_SLAVE_DISABLE |
> + DW_IC_CON_RESTART_EN | DW_IC_CON_SPEED_STD;
> + else
> + dev->master_cfg = DW_IC_CON_MASTER | DW_IC_CON_SLAVE_DISABLE |
> + DW_IC_CON_RESTART_EN | DW_IC_CON_SPEED_FAST;
>
> /* Try first if we can configure the device from ACPI */
> r = dw_i2c_acpi_configure(pdev);
> --
> 1.7.1
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] i2c designware add support of I2C standard mode
From: Romain Baeriswyl
Some legacy devices support ony I2C standard mode at 100kHz.
This patch allows to select the standard mode through the DTS
with the use of the existing clock-frequency parameter.
When clock-frequency parameter is not set, the fast mode is selected.
Only when the parameter is set at 10, the standard mode is selected.
Signed-off-by: Romain Baeriswyl
Reviewed-by: Christian Ruppert
---
drivers/i2c/busses/i2c-designware-platdrv.c | 23 +--
1 files changed, 21 insertions(+), 2 deletions(-)
diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c
b/drivers/i2c/busses/i2c-designware-platdrv.c
index 402ec39..b543fe1 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -122,6 +122,7 @@ static int dw_i2c_probe(struct platform_device *pdev)
struct i2c_adapter *adap;
struct resource *mem;
int irq, r;
+ u32 clk_freq;
irq = platform_get_irq(pdev, 0);
if (irq < 0) {
@@ -151,6 +152,9 @@ static int dw_i2c_probe(struct platform_device *pdev)
return PTR_ERR(dev->clk);
clk_prepare_enable(dev->clk);
+ /* fast mode by default because of legacy reasons */
+ clk_freq = 40;
+
if (pdev->dev.of_node) {
u32 ht = 0;
u32 ic_clk = dev->get_clk_rate_khz(dev);
@@ -166,6 +170,17 @@ static int dw_i2c_probe(struct platform_device *pdev)
of_property_read_u32(pdev->dev.of_node,
"i2c-scl-falling-time-ns",
&dev->scl_falling_time);
+
+ of_property_read_u32(pdev->dev.of_node, "clock-frequency",
+&clk_freq);
+
+ /* Only standard mode at 100kHz and fast mode at 400kHz
+* are supported.
+*/
+ if (clk_freq != 10 && clk_freq != 40) {
+ dev_err(&pdev->dev, "Only 100kHz and 400kHz supported");
+ return -EINVAL;
+ }
}
dev->functionality =
@@ -175,8 +190,12 @@ static int dw_i2c_probe(struct platform_device *pdev)
I2C_FUNC_SMBUS_BYTE_DATA |
I2C_FUNC_SMBUS_WORD_DATA |
I2C_FUNC_SMBUS_I2C_BLOCK;
- dev->master_cfg = DW_IC_CON_MASTER | DW_IC_CON_SLAVE_DISABLE |
- DW_IC_CON_RESTART_EN | DW_IC_CON_SPEED_FAST;
+ if (clk_freq == 10)
+ dev->master_cfg = DW_IC_CON_MASTER | DW_IC_CON_SLAVE_DISABLE |
+ DW_IC_CON_RESTART_EN | DW_IC_CON_SPEED_STD;
+ else
+ dev->master_cfg = DW_IC_CON_MASTER | DW_IC_CON_SLAVE_DISABLE |
+ DW_IC_CON_RESTART_EN | DW_IC_CON_SPEED_FAST;
/* Try first if we can configure the device from ACPI */
r = dw_i2c_acpi_configure(pdev);
--
1.7.1
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] i2c: designware: deduce speed mode from device tree setting
On Wed, 20 Aug 2014, Romain Baeriswyl wrote:
> Hi,
>
> With the patch "i2c designware add support of I2C standard mode" I already
> proposed:
> - I2C standard mode is selected with 100kHz clock frequency.
> - I2C fast mode is selected with 400kHy clock frequency.
> - EINVAL error is returned if clock frequency is not 10 and not 40.
>
> but this patch seems not available yet.
> What about the other patch "i2c designware make SCL and SDA falling time
> configurable" ?
>
> In i2c-designware_platdrv.c the flag DW_IC_CON_SPEED_xxx is well set
> depending on the mode:
>
> if (clk_freq == 10)
Romain,
I'm really happy if your patches get accepted. Can this be <= 10?
It is really common to run I2C at a lower speed if you have some
board issues with the i2c bus.
Alan
> dev->master_cfg = DW_IC_CON_MASTER | DW_IC_CON_SLAVE_DISABLE |
>DW_IC_CON_RESTART_EN | DW_IC_CON_SPEED_STD;
>else
> dev->master_cfg = DW_IC_CON_MASTER | DW_IC_CON_SLAVE_DISABLE |
>DW_IC_CON_RESTART_EN | DW_IC_CON_SPEED_FAST;
>
>
> So for me everything should be fine if there patches are applied.
>
> Regards,
>
> Romain
>
> - Original Message -
> From: "Mark Rutland"
> To: at...@opensource.altera.com
> Cc: w...@the-dreams.de, bar...@tkos.co.il, "mika westerberg"
> , "grant likely" ,
> robh...@kernel.org, skuri...@pobox.com, "Romain Baeriswyl"
> , "rafael j wysocki"
> , a...@linux.intel.com,
> linux-i2c@vger.kernel.org, linux-ker...@vger.kernel.org,
> devicet...@vger.kernel.org, "delicious quinoa" ,
> dingu...@opensource.altera.com, yvand...@opensource.altera.com
> Sent: Wednesday, August 20, 2014 11:22:57 AM
> Subject: Re: [PATCH] i2c: designware: deduce speed mode from device tree
> setting
>
> On Tue, Aug 19, 2014 at 09:18:49PM +0100, at...@opensource.altera.com wrote:
> > From: Alan Tull
> >
> > Use the documented, but unimplemented "clock-frequency"
> > Device Tree setting as a guide on whether to set the speed
> > mode bits in DW_IC_CON to standard or fast i2c mode.
> >
> > Previously, the driver was hardwired to fast mode. Default
> > to fast mode if the "clock-frequency" property is not present
> > for backwards compatiblity.
> >
> > Signed-off-by: Alan Tull
> > ---
> > drivers/i2c/busses/i2c-designware-platdrv.c | 10 --
> > 1 file changed, 8 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c
> > b/drivers/i2c/busses/i2c-designware-platdrv.c
> > index bc87733..18cd3d9 100644
> > --- a/drivers/i2c/busses/i2c-designware-platdrv.c
> > +++ b/drivers/i2c/busses/i2c-designware-platdrv.c
> > @@ -122,7 +122,8 @@ static int dw_i2c_probe(struct platform_device *pdev)
> > struct dw_i2c_dev *dev;
> > struct i2c_adapter *adap;
> > struct resource *mem;
> > - int irq, r;
> > + int irq, r, ret, speed = DW_IC_CON_SPEED_FAST;
> > + u32 bus_rate;
> >
> > irq = platform_get_irq(pdev, 0);
> > if (irq < 0) {
> > @@ -167,6 +168,11 @@ static int dw_i2c_probe(struct platform_device *pdev)
> > of_property_read_u32(pdev->dev.of_node,
> > "i2c-scl-falling-time-ns",
> > &dev->scl_falling_time);
> > +
> > + ret = of_property_read_u32(pdev->dev.of_node,
> > + "clock-frequency", &bus_rate);
> > + if (!ret && (bus_rate <= 10))
> > + speed = DW_IC_CON_SPEED_STD;
>
> This looks a bit odd.
>
> If the device only supports two particular speeds why do we accept any
> other speed in the clock-frequency property? Surely we should at least
> warn that something was off?
>
> Thanks,
> Mark
>
> > }
> >
> > dev->functionality =
> > @@ -177,7 +183,7 @@ static int dw_i2c_probe(struct platform_device *pdev)
> > I2C_FUNC_SMBUS_WORD_DATA |
> > I2C_FUNC_SMBUS_I2C_BLOCK;
> > dev->master_cfg = DW_IC_CON_MASTER | DW_IC_CON_SLAVE_DISABLE |
> > - DW_IC_CON_RESTART_EN | DW_IC_CON_SPEED_FAST;
> > + DW_IC_CON_RESTART_EN | speed;
> >
> > /* Try first if we can configure the device from ACPI */
> > r = dw_i2c_acpi_configure(pdev);
> > --
> > 1.7.9.5
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe devicetree" in
> > the body of a message to majord...@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> >
>
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RESEND PATCH 3/7] mfd: cros_ec: stop calling ->cmd_xfer() directly
From: Andrew Bresticker
Instead of having users of the ChromeOS EC call the interface-specific
cmd_xfer() callback directly, introduce a central cros_ec_cmd_xfer()
to use instead. This will allow us to put all the locking and retry
logic in one place instead of duplicating it across the different
drivers.
Signed-off-by: Andrew Bresticker
Reviewed-by: Simon Glass
Signed-off-by: Javier Martinez Canillas
---
drivers/i2c/busses/i2c-cros-ec-tunnel.c | 2 +-
drivers/input/keyboard/cros_ec_keyb.c | 2 +-
drivers/mfd/cros_ec.c | 7 +++
include/linux/mfd/cros_ec.h | 24 ++--
4 files changed, 27 insertions(+), 8 deletions(-)
diff --git a/drivers/i2c/busses/i2c-cros-ec-tunnel.c
b/drivers/i2c/busses/i2c-cros-ec-tunnel.c
index a4411da..8ca5cbb 100644
--- a/drivers/i2c/busses/i2c-cros-ec-tunnel.c
+++ b/drivers/i2c/busses/i2c-cros-ec-tunnel.c
@@ -229,7 +229,7 @@ static int ec_i2c_xfer(struct i2c_adapter *adap, struct
i2c_msg i2c_msgs[],
msg.indata = response;
msg.insize = response_len;
- result = bus->ec->cmd_xfer(bus->ec, &msg);
+ result = cros_ec_cmd_xfer(bus->ec, &msg);
if (result < 0)
goto exit;
diff --git a/drivers/input/keyboard/cros_ec_keyb.c
b/drivers/input/keyboard/cros_ec_keyb.c
index 791781a..93111d1 100644
--- a/drivers/input/keyboard/cros_ec_keyb.c
+++ b/drivers/input/keyboard/cros_ec_keyb.c
@@ -182,7 +182,7 @@ static int cros_ec_keyb_get_state(struct cros_ec_keyb
*ckdev, uint8_t *kb_state)
.insize = ckdev->cols,
};
- return ckdev->ec->cmd_xfer(ckdev->ec, &msg);
+ return cros_ec_cmd_xfer(ckdev->ec, &msg);
}
static irqreturn_t cros_ec_keyb_irq(int irq, void *data)
diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c
index 4873f9c..a9faebd 100644
--- a/drivers/mfd/cros_ec.c
+++ b/drivers/mfd/cros_ec.c
@@ -62,6 +62,13 @@ int cros_ec_check_result(struct cros_ec_device *ec_dev,
}
EXPORT_SYMBOL(cros_ec_check_result);
+int cros_ec_cmd_xfer(struct cros_ec_device *ec_dev,
+struct cros_ec_command *msg)
+{
+ return ec_dev->cmd_xfer(ec_dev, msg);
+}
+EXPORT_SYMBOL(cros_ec_cmd_xfer);
+
static const struct mfd_cell cros_devs[] = {
{
.name = "cros-ec-keyb",
diff --git a/include/linux/mfd/cros_ec.h b/include/linux/mfd/cros_ec.h
index fcbe9d1..0e166b9 100644
--- a/include/linux/mfd/cros_ec.h
+++ b/include/linux/mfd/cros_ec.h
@@ -62,10 +62,6 @@ struct cros_ec_command {
* @dev: Device pointer
* @was_wake_device: true if this device was set to wake the system from
* sleep at the last suspend
- * @cmd_xfer: send command to EC and get response
- * Returns the number of bytes received if the communication succeeded, but
- * that doesn't mean the EC was happy with the command. The caller
- * should check msg.result for the EC's result code.
*
* @priv: Private data
* @irq: Interrupt to use
@@ -82,6 +78,10 @@ struct cros_ec_command {
* @dout_size: size of dout buffer to allocate (zero to use static dout)
* @parent: pointer to parent device (e.g. i2c or spi device)
* @wake_enabled: true if this device can wake the system from sleep
+ * @cmd_xfer: send command to EC and get response
+ * Returns the number of bytes received if the communication succeeded, but
+ * that doesn't mean the EC was happy with the command. The caller
+ * should check msg.result for the EC's result code.
* @lock: one transaction at a time
*/
struct cros_ec_device {
@@ -92,8 +92,6 @@ struct cros_ec_device {
struct device *dev;
bool was_wake_device;
struct class *cros_class;
- int (*cmd_xfer)(struct cros_ec_device *ec,
- struct cros_ec_command *msg);
/* These are used to implement the platform-specific interface */
void *priv;
@@ -104,6 +102,8 @@ struct cros_ec_device {
int dout_size;
struct device *parent;
bool wake_enabled;
+ int (*cmd_xfer)(struct cros_ec_device *ec,
+ struct cros_ec_command *msg);
struct mutex lock;
};
@@ -153,6 +153,18 @@ int cros_ec_check_result(struct cros_ec_device *ec_dev,
struct cros_ec_command *msg);
/**
+ * cros_ec_cmd_xfer - Send a command to the ChromeOS EC
+ *
+ * Call this to send a command to the ChromeOS EC. This should be used
+ * instead of calling the EC's cmd_xfer() callback directly.
+ *
+ * @ec_dev: EC device
+ * @msg: Message to write
+ */
+int cros_ec_cmd_xfer(struct cros_ec_device *ec_dev,
+struct cros_ec_command *msg);
+
+/**
* cros_ec_remove - Remove a ChromeOS EC
*
* Call this to deregister a ChromeOS EC, then clean up any private data.
--
2.0.1
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] i2c: designware: deduce speed mode from device tree setting
> > + > > + ret = of_property_read_u32(pdev->dev.of_node, > > + "clock-frequency", &bus_rate); > > + if (!ret && (bus_rate <= 10)) > > + speed = DW_IC_CON_SPEED_STD; > > This looks a bit odd. > > If the device only supports two particular speeds why do we accept any > other speed in the clock-frequency property? "clock-frequency" is the default binding for specifying i2c bus speeds today. Some controllers can be programmed to do various speeds, some can only do a set of fixed values. > Surely we should at least warn that something was off? Yes, I was going to say the same until Romain's old patch showed up which does that. Thanks, Wolfram signature.asc Description: Digital signature
Re: [PATCH] i2c: designware: deduce speed mode from device tree setting
> With the patch "i2c designware add support of I2C standard mode" I already > proposed: > - I2C standard mode is selected with 100kHz clock frequency. > - I2C fast mode is selected with 400kHy clock frequency. > - EINVAL error is returned if clock frequency is not 10 and not 40. You forgot to send this patch to the i2c list. So, it didn't get into patchwork which I rely on for tracking patches. Please resend an updated version to the latest codebase. I like that it has the EINVAL error checking. > What about the other patch "i2c designware make SCL and SDA falling time > configurable" ? 6468276b22069d4442aafcd8c59e5d8ccae23f5f upstream. signature.asc Description: Digital signature
Re: [PATCH] i2c: designware: deduce speed mode from device tree setting
On Wed, Aug 20, 2014 at 01:36:18PM +0100, Wolfram Sang wrote: > > > + > > > + ret = of_property_read_u32(pdev->dev.of_node, > > > +"clock-frequency", &bus_rate); > > > + if (!ret && (bus_rate <= 10)) > > > + speed = DW_IC_CON_SPEED_STD; > > > > This looks a bit odd. > > > > If the device only supports two particular speeds why do we accept any > > other speed in the clock-frequency property? > > "clock-frequency" is the default binding for specifying i2c bus speeds > today. Some controllers can be programmed to do various speeds, some can > only do a set of fixed values. Sure. My complaint was that the driver would accept invalid values. That wasn't meant to be suggestion to use a property other than the standard clock-freqeuncy. > > Surely we should at least warn that something was off? > > Yes, I was going to say the same until Romain's old patch showed up > which does that. Cool. Sounds like we can use Romain's patch to handle this then. Cheers, Mark. -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RESEND PATCH 0/7] Second batch of cleanups for cros_ec
Hello Lee, This is a resend of a patch series originally sent [0] almost a month ago (July, 28). The series add a batch of cleanups patches for the mfd cros_ec driver and its subdevices drivers. The first batch of cleanups was posted by Doug Anderson [1] and have already been merged. The patches were taken from the ChromeOS 3.8 kernel and after this series, no cleanups patches for cros_ec are left. The remaining commits add support not yet available in mainline. There is almost no functionality added on this series but the idea is to reduce the delta between the mainline drivers and the ones in the downstream Chrome OS 3.8 kernel so the missing functionality can be added on top once these cleanups patches are merged. The missing functionlity currently in mainline is: - Chrome OS Embedded Controller userspace device interface - Chrome OS Embedded Controller Low Pin Count (LPC) inteface - Access to vboot context stored on a block device - Access to vboot context stored on EC's nvram The patches in this series are authored by different people (all on cc) and consist of the following: Andrew Bresticker (3): mfd: cros_ec: stop calling ->cmd_xfer() directly mfd: cros_ec: move locking into cros_ec_cmd_xfer mfd: cros_ec: wait for completion of commands that return IN_PROGRESS Derek Basehore (1): i2c: i2c-cros-ec-tunnel: Set retries to 3 Doug Anderson (1): mfd: cros_ec: Delay for 50ms when we see EC_CMD_REBOOT_EC Todd Broch (2): mfd: cros_ec: Instantiate sub-devices from device tree Input: cros_ec_keyb: Optimize ghosting algorithm. drivers/i2c/busses/i2c-cros-ec-tunnel.c | 5 +- drivers/input/keyboard/cros_ec_keyb.c | 89 + drivers/mfd/cros_ec.c | 88 drivers/mfd/cros_ec_spi.c | 20 include/linux/mfd/cros_ec.h | 24 ++--- 5 files changed, 154 insertions(+), 72 deletions(-) There were no changes on this resend, just picked Acked-by and Tested-by tags and also added my own Signed-off-by tag to all the patches as suggested by Andreas Färber even when I just picked them from downstream and rebased on top of linux-next. The patches should be merged together which means that they should go through your mfd tree once the relevant acks are obtained. Best regards, Javier [0]: https://www.mail-archive.com/linux-input@vger.kernel.org/msg11385.html [1]: https://lkml.org/lkml/2014/6/16/681 -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RESEND PATCH 4/7] mfd: cros_ec: move locking into cros_ec_cmd_xfer
From: Andrew Bresticker
Now that there's a central cros_ec_cmd_xfer(), move the locking
out of the SPI and LPC drivers.
Signed-off-by: Andrew Bresticker
Reviewed-by: Simon Glass
Signed-off-by: Javier Martinez Canillas
---
drivers/mfd/cros_ec.c | 10 +-
drivers/mfd/cros_ec_spi.c | 11 ---
2 files changed, 9 insertions(+), 12 deletions(-)
diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c
index a9faebd..c53804a 100644
--- a/drivers/mfd/cros_ec.c
+++ b/drivers/mfd/cros_ec.c
@@ -65,7 +65,13 @@ EXPORT_SYMBOL(cros_ec_check_result);
int cros_ec_cmd_xfer(struct cros_ec_device *ec_dev,
struct cros_ec_command *msg)
{
- return ec_dev->cmd_xfer(ec_dev, msg);
+ int ret;
+
+ mutex_lock(&ec_dev->lock);
+ ret = ec_dev->cmd_xfer(ec_dev, msg);
+ mutex_unlock(&ec_dev->lock);
+
+ return ret;
}
EXPORT_SYMBOL(cros_ec_cmd_xfer);
@@ -98,6 +104,8 @@ int cros_ec_register(struct cros_ec_device *ec_dev)
return -ENOMEM;
}
+ mutex_init(&ec_dev->lock);
+
err = mfd_add_devices(dev, 0, cros_devs,
ARRAY_SIZE(cros_devs),
NULL, ec_dev->irq, NULL);
diff --git a/drivers/mfd/cros_ec_spi.c b/drivers/mfd/cros_ec_spi.c
index b396705..bf6e08e 100644
--- a/drivers/mfd/cros_ec_spi.c
+++ b/drivers/mfd/cros_ec_spi.c
@@ -79,13 +79,11 @@
* if no record
* @end_of_msg_delay: used to set the delay_usecs on the spi_transfer that
* is sent when we want to turn off CS at the end of a transaction.
- * @lock: mutex to ensure only one user of cros_ec_cmd_xfer_spi at a time
*/
struct cros_ec_spi {
struct spi_device *spi;
s64 last_transfer_ns;
unsigned int end_of_msg_delay;
- struct mutex lock;
};
static void debug_packet(struct device *dev, const char *name, u8 *ptr,
@@ -232,13 +230,6 @@ static int cros_ec_cmd_xfer_spi(struct cros_ec_device
*ec_dev,
int sum;
int ret = 0, final_ret;
- /*
-* We have the shared ec_dev buffer plus we do lots of separate spi_sync
-* calls, so we need to make sure only one person is using this at a
-* time.
-*/
- mutex_lock(&ec_spi->lock);
-
len = cros_ec_prepare_tx(ec_dev, ec_msg);
dev_dbg(ec_dev->dev, "prepared, len=%d\n", len);
@@ -327,7 +318,6 @@ exit:
if (ec_msg->command == EC_CMD_REBOOT_EC)
msleep(EC_REBOOT_DELAY_MS);
- mutex_unlock(&ec_spi->lock);
return ret;
}
@@ -359,7 +349,6 @@ static int cros_ec_spi_probe(struct spi_device *spi)
if (ec_spi == NULL)
return -ENOMEM;
ec_spi->spi = spi;
- mutex_init(&ec_spi->lock);
ec_dev = devm_kzalloc(dev, sizeof(*ec_dev), GFP_KERNEL);
if (!ec_dev)
return -ENOMEM;
--
2.0.1
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RESEND PATCH 2/7] i2c: i2c-cros-ec-tunnel: Set retries to 3
From: Derek Basehore
Since the i2c bus can get wedged on the EC sometimes, set the number of retries
to 3. Since we un-wedge the bus immediately after the wedge happens, this is the
correct fix since only one transfer will fail.
Signed-off-by: Derek Basehore
Reviewed-by: Doug Anderson
Acked-by: Wolfram Sang
Signed-off-by: Javier Martinez Canillas
---
drivers/i2c/busses/i2c-cros-ec-tunnel.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/i2c/busses/i2c-cros-ec-tunnel.c
b/drivers/i2c/busses/i2c-cros-ec-tunnel.c
index 05e033c..a4411da 100644
--- a/drivers/i2c/busses/i2c-cros-ec-tunnel.c
+++ b/drivers/i2c/busses/i2c-cros-ec-tunnel.c
@@ -16,6 +16,8 @@
#include
#include
+#define I2C_MAX_RETRIES 3
+
/**
* struct ec_i2c_device - Driver data for I2C tunnel
*
@@ -290,6 +292,7 @@ static int ec_i2c_probe(struct platform_device *pdev)
bus->adap.algo_data = bus;
bus->adap.dev.parent = &pdev->dev;
bus->adap.dev.of_node = np;
+ bus->adap.retries = I2C_MAX_RETRIES;
err = i2c_add_adapter(&bus->adap);
if (err) {
--
2.0.1
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RESEND PATCH 5/7] mfd: cros_ec: wait for completion of commands that return IN_PROGRESS
From: Andrew Bresticker
When an EC command returns EC_RES_IN_PROGRESS, we need to query
the state of the EC until it indicates that it is no longer busy.
Do this in cros_ec_cmd_xfer() under the EC's mutex so that other
commands (e.g. keyboard, I2C passtru) aren't issued to the EC while
it is working on the in-progress command.
Signed-off-by: Andrew Bresticker
Reviewed-by: Simon Glass
Signed-off-by: Javier Martinez Canillas
---
drivers/mfd/cros_ec.c | 35 ++-
1 file changed, 34 insertions(+), 1 deletion(-)
diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c
index c53804a..634c434 100644
--- a/drivers/mfd/cros_ec.c
+++ b/drivers/mfd/cros_ec.c
@@ -23,6 +23,10 @@
#include
#include
#include
+#include
+
+#define EC_COMMAND_RETRIES 50
+#define EC_RETRY_DELAY_MS 10
int cros_ec_prepare_tx(struct cros_ec_device *ec_dev,
struct cros_ec_command *msg)
@@ -65,10 +69,39 @@ EXPORT_SYMBOL(cros_ec_check_result);
int cros_ec_cmd_xfer(struct cros_ec_device *ec_dev,
struct cros_ec_command *msg)
{
- int ret;
+ int ret, i;
mutex_lock(&ec_dev->lock);
ret = ec_dev->cmd_xfer(ec_dev, msg);
+ if (ret == -EAGAIN && msg->result == EC_RES_IN_PROGRESS) {
+ /*
+* Query the EC's status until it's no longer busy or
+* we encounter an error.
+*/
+ for (i = 0; i < EC_COMMAND_RETRIES; i++) {
+ struct cros_ec_command status_msg;
+ struct ec_response_get_comms_status status;
+
+ msleep(EC_RETRY_DELAY_MS);
+
+ status_msg.version = 0;
+ status_msg.command = EC_CMD_GET_COMMS_STATUS;
+ status_msg.outdata = NULL;
+ status_msg.outsize = 0;
+ status_msg.indata = (uint8_t *)&status;
+ status_msg.insize = sizeof(status);
+
+ ret = ec_dev->cmd_xfer(ec_dev, &status_msg);
+ if (ret < 0)
+ break;
+
+ msg->result = status_msg.result;
+ if (status_msg.result != EC_RES_SUCCESS)
+ break;
+ if (!(status.flags & EC_COMMS_STATUS_PROCESSING))
+ break;
+ }
+ }
mutex_unlock(&ec_dev->lock);
return ret;
--
2.0.1
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RESEND PATCH 1/7] mfd: cros_ec: Delay for 50ms when we see EC_CMD_REBOOT_EC
From: Doug Anderson If someone sends a EC_CMD_REBOOT_EC to the EC, the EC will likely be unresponsive for quite a while. Add a delay to the end of the command to prevent random failures of future commands. NOTES: * This could be optimized a bit by simply delaying the next command sent, but EC_CMD_REBOOT_EC is such a rare command that the extra complexity doesn't seem worth it. * This is a bit of an "ugly hack" since the SPI driver is effectively snooping on the communication and making a lot of assumptions. It would be nice to architect in some better solution long term. * This same logic probably needs to be applied to the i2c driver. Signed-off-by: Doug Anderson Reviewed-by: Randall Spangler Reviewed-by: Vadim Bendebury Signed-off-by: Javier Martinez Canillas --- drivers/mfd/cros_ec_spi.c | 9 + 1 file changed, 9 insertions(+) diff --git a/drivers/mfd/cros_ec_spi.c b/drivers/mfd/cros_ec_spi.c index 588c700..b396705 100644 --- a/drivers/mfd/cros_ec_spi.c +++ b/drivers/mfd/cros_ec_spi.c @@ -65,6 +65,12 @@ */ #define EC_SPI_RECOVERY_TIME_NS(200 * 1000) +/* + * The EC is unresponsive for a time after a reboot command. Add a + * simple delay to make sure that the bus stays locked. + */ +#define EC_REBOOT_DELAY_MS 50 + /** * struct cros_ec_spi - information about a SPI-connected EC * @@ -318,6 +324,9 @@ static int cros_ec_cmd_xfer_spi(struct cros_ec_device *ec_dev, ret = len; exit: + if (ec_msg->command == EC_CMD_REBOOT_EC) + msleep(EC_REBOOT_DELAY_MS); + mutex_unlock(&ec_spi->lock); return ret; } -- 2.0.1 -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RESEND PATCH 7/7] Input: cros_ec_keyb: Optimize ghosting algorithm.
From: Todd Broch
Previous algorithm was a bit conservative and complicating with
respect to identifying key ghosting. This CL uses the bitops hamming
weight function (hweight8) to count the number of matching rows for
colM & colN. If that number is > 1 ghosting is present.
Additionally it removes NULL keys and our one virtual keypress
KEY_BATTERY from consideration as these inputs are never physical
keypresses.
Signed-off-by: Todd Broch
Reviewed-by: Vincent Palatin
Reviewed-by: Luigi Semenzato
Tested-by: Andreas Färber
Signed-off-by: Javier Martinez Canillas
---
drivers/input/keyboard/cros_ec_keyb.c | 92 +++
1 file changed, 49 insertions(+), 43 deletions(-)
diff --git a/drivers/input/keyboard/cros_ec_keyb.c
b/drivers/input/keyboard/cros_ec_keyb.c
index 93111d1..5d773d2 100644
--- a/drivers/input/keyboard/cros_ec_keyb.c
+++ b/drivers/input/keyboard/cros_ec_keyb.c
@@ -22,6 +22,7 @@
*/
#include
+#include
#include
#include
#include
@@ -38,6 +39,7 @@
* @row_shift: log2 or number of rows, rounded up
* @keymap_data: Matrix keymap data used to convert to keyscan values
* @ghost_filter: true to enable the matrix key-ghosting filter
+ * @valid_keys: bitmap of existing keys for each matrix column
* @old_kb_state: bitmap of keys pressed last scan
* @dev: Device pointer
* @idev: Input device
@@ -49,6 +51,7 @@ struct cros_ec_keyb {
int row_shift;
const struct matrix_keymap_data *keymap_data;
bool ghost_filter;
+ uint8_t *valid_keys;
uint8_t *old_kb_state;
struct device *dev;
@@ -57,39 +60,15 @@ struct cros_ec_keyb {
};
-static bool cros_ec_keyb_row_has_ghosting(struct cros_ec_keyb *ckdev,
- uint8_t *buf, int row)
-{
- int pressed_in_row = 0;
- int row_has_teeth = 0;
- int col, mask;
-
- mask = 1 << row;
- for (col = 0; col < ckdev->cols; col++) {
- if (buf[col] & mask) {
- pressed_in_row++;
- row_has_teeth |= buf[col] & ~mask;
- if (pressed_in_row > 1 && row_has_teeth) {
- /* ghosting */
- dev_dbg(ckdev->dev,
- "ghost found at: r%d c%d, pressed %d,
teeth 0x%x\n",
- row, col, pressed_in_row,
- row_has_teeth);
- return true;
- }
- }
- }
-
- return false;
-}
-
/*
* Returns true when there is at least one combination of pressed keys that
* results in ghosting.
*/
static bool cros_ec_keyb_has_ghosting(struct cros_ec_keyb *ckdev, uint8_t *buf)
{
- int row;
+ int col1, col2, buf1, buf2;
+ struct device *dev = ckdev->dev;
+ uint8_t *valid_keys = ckdev->valid_keys;
/*
* Ghosting happens if for any pressed key X there are other keys
@@ -103,27 +82,23 @@ static bool cros_ec_keyb_has_ghosting(struct cros_ec_keyb
*ckdev, uint8_t *buf)
*
* In this case only X, Y, and Z are pressed, but g appears to be
* pressed too (see Wikipedia).
-*
-* We can detect ghosting in a single pass (*) over the keyboard state
-* by maintaining two arrays. pressed_in_row counts how many pressed
-* keys we have found in a row. row_has_teeth is true if any of the
-* pressed keys for this row has other pressed keys in its column. If
-* at any point of the scan we find that a row has multiple pressed
-* keys, and at least one of them is at the intersection with a column
-* with multiple pressed keys, we're sure there is ghosting.
-* Conversely, if there is ghosting, we will detect such situation for
-* at least one key during the pass.
-*
-* (*) This looks linear in the number of keys, but it's not. We can
-* cheat because the number of rows is small.
*/
- for (row = 0; row < ckdev->rows; row++)
- if (cros_ec_keyb_row_has_ghosting(ckdev, buf, row))
- return true;
+ for (col1 = 0; col1 < ckdev->cols; col1++) {
+ buf1 = buf[col1] & valid_keys[col1];
+ for (col2 = col1 + 1; col2 < ckdev->cols; col2++) {
+ buf2 = buf[col2] & valid_keys[col2];
+ if (hweight8(buf1 & buf2) > 1) {
+ dev_dbg(dev, "ghost found at: B[%02d]:0x%02x &
B[%02d]:0x%02x",
+ col1, buf1, col2, buf2);
+ return true;
+ }
+ }
+ }
return false;
}
+
/*
* Compares the new keyboard state to the old one and produces key
* press/release events accordingly. The keyboard state is 13 bytes (one byte
@@ -222,6 +197,30 @@ st
Re: [PATCH] i2c: designware: deduce speed mode from device tree setting
Please find below link to these patches:
[PATCH 1/2] i2c designware make SCL and SDA falling time configurable
https://lkml.org/lkml/2013/10/8/443
[PATCH V3 2/2] i2c designware add support of I2C standard mode
https://lkml.org/lkml/2014/3/25/135
Are they fine to be included in the mainline ?
Romain
- Original Message -
From: "Romain Baeriswyl"
To: "Mark Rutland" , at...@opensource.altera.com
Cc: w...@the-dreams.de, bar...@tkos.co.il, "mika westerberg"
, "grant likely" ,
robh...@kernel.org, skuri...@pobox.com, "rafael j wysocki"
, a...@linux.intel.com, linux-i2c@vger.kernel.org,
linux-ker...@vger.kernel.org, devicet...@vger.kernel.org, "delicious quinoa"
, dingu...@opensource.altera.com,
yvand...@opensource.altera.com
Sent: Wednesday, August 20, 2014 11:49:37 AM
Subject: Re: [PATCH] i2c: designware: deduce speed mode from device tree setting
Hi,
With the patch "i2c designware add support of I2C standard mode" I already
proposed:
- I2C standard mode is selected with 100kHz clock frequency.
- I2C fast mode is selected with 400kHy clock frequency.
- EINVAL error is returned if clock frequency is not 10 and not 40.
but this patch seems not available yet.
What about the other patch "i2c designware make SCL and SDA falling time
configurable" ?
In i2c-designware_platdrv.c the flag DW_IC_CON_SPEED_xxx is well set
depending on the mode:
if (clk_freq == 10)
dev->master_cfg = DW_IC_CON_MASTER | DW_IC_CON_SLAVE_DISABLE |
DW_IC_CON_RESTART_EN | DW_IC_CON_SPEED_STD;
else
dev->master_cfg = DW_IC_CON_MASTER | DW_IC_CON_SLAVE_DISABLE |
DW_IC_CON_RESTART_EN | DW_IC_CON_SPEED_FAST;
So for me everything should be fine if there patches are applied.
Regards,
Romain
- Original Message -
From: "Mark Rutland"
To: at...@opensource.altera.com
Cc: w...@the-dreams.de, bar...@tkos.co.il, "mika westerberg"
, "grant likely" ,
robh...@kernel.org, skuri...@pobox.com, "Romain Baeriswyl"
, "rafael j wysocki" ,
a...@linux.intel.com, linux-i2c@vger.kernel.org, linux-ker...@vger.kernel.org,
devicet...@vger.kernel.org, "delicious quinoa" ,
dingu...@opensource.altera.com, yvand...@opensource.altera.com
Sent: Wednesday, August 20, 2014 11:22:57 AM
Subject: Re: [PATCH] i2c: designware: deduce speed mode from device tree setting
On Tue, Aug 19, 2014 at 09:18:49PM +0100, at...@opensource.altera.com wrote:
> From: Alan Tull
>
> Use the documented, but unimplemented "clock-frequency"
> Device Tree setting as a guide on whether to set the speed
> mode bits in DW_IC_CON to standard or fast i2c mode.
>
> Previously, the driver was hardwired to fast mode. Default
> to fast mode if the "clock-frequency" property is not present
> for backwards compatiblity.
>
> Signed-off-by: Alan Tull
> ---
> drivers/i2c/busses/i2c-designware-platdrv.c | 10 --
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c
> b/drivers/i2c/busses/i2c-designware-platdrv.c
> index bc87733..18cd3d9 100644
> --- a/drivers/i2c/busses/i2c-designware-platdrv.c
> +++ b/drivers/i2c/busses/i2c-designware-platdrv.c
> @@ -122,7 +122,8 @@ static int dw_i2c_probe(struct platform_device *pdev)
> struct dw_i2c_dev *dev;
> struct i2c_adapter *adap;
> struct resource *mem;
> - int irq, r;
> + int irq, r, ret, speed = DW_IC_CON_SPEED_FAST;
> + u32 bus_rate;
>
> irq = platform_get_irq(pdev, 0);
> if (irq < 0) {
> @@ -167,6 +168,11 @@ static int dw_i2c_probe(struct platform_device *pdev)
> of_property_read_u32(pdev->dev.of_node,
>"i2c-scl-falling-time-ns",
>&dev->scl_falling_time);
> +
> + ret = of_property_read_u32(pdev->dev.of_node,
> +"clock-frequency", &bus_rate);
> + if (!ret && (bus_rate <= 10))
> + speed = DW_IC_CON_SPEED_STD;
This looks a bit odd.
If the device only supports two particular speeds why do we accept any
other speed in the clock-frequency property? Surely we should at least
warn that something was off?
Thanks,
Mark
> }
>
> dev->functionality =
> @@ -177,7 +183,7 @@ static int dw_i2c_probe(struct platform_device *pdev)
> I2C_FUNC_SMBUS_WORD_DATA |
> I2C_FUNC_SMBUS_I2C_BLOCK;
> dev->master_cfg = DW_IC_CON_MASTER | DW_IC_CON_SLAVE_DISABLE |
> - DW_IC_CON_RESTART_EN | DW_IC_CON_SPEED_FAST;
> + DW_IC_CON_RESTART_EN | speed;
>
> /* Try first if we can configure the device from ACPI */
> r = dw_i2c_acpi_configure(pdev);
> --
> 1.7.9.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body o
[RESEND PATCH 6/7] mfd: cros_ec: Instantiate sub-devices from device tree
From: Todd Broch
If the EC device tree node has sub-nodes, try to instantiate them as
MFD sub-devices. We can configure the EC features provided by the board.
Signed-off-by: Todd Broch
Signed-off-by: Javier Martinez Canillas
---
drivers/mfd/cros_ec.c | 40 ++--
1 file changed, 30 insertions(+), 10 deletions(-)
diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c
index 634c434..96c926c 100644
--- a/drivers/mfd/cros_ec.c
+++ b/drivers/mfd/cros_ec.c
@@ -21,6 +21,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -109,22 +110,16 @@ int cros_ec_cmd_xfer(struct cros_ec_device *ec_dev,
EXPORT_SYMBOL(cros_ec_cmd_xfer);
static const struct mfd_cell cros_devs[] = {
- {
- .name = "cros-ec-keyb",
- .id = 1,
- .of_compatible = "google,cros-ec-keyb",
- },
- {
- .name = "cros-ec-i2c-tunnel",
- .id = 2,
- .of_compatible = "google,cros-ec-i2c-tunnel",
- },
};
int cros_ec_register(struct cros_ec_device *ec_dev)
{
struct device *dev = ec_dev->dev;
int err = 0;
+#ifdef CONFIG_OF
+ struct device_node *node;
+ int id = ARRAY_SIZE(cros_devs);
+#endif
if (ec_dev->din_size) {
ec_dev->din = devm_kzalloc(dev, ec_dev->din_size, GFP_KERNEL);
@@ -146,6 +141,31 @@ int cros_ec_register(struct cros_ec_device *ec_dev)
dev_err(dev, "failed to add mfd devices\n");
return err;
}
+#ifdef CONFIG_OF
+ /*
+* Add sub-devices declared in the device tree. NOTE they should NOT be
+* declared in cros_devs
+*/
+ for_each_child_of_node(dev->of_node, node) {
+ char name[128];
+ struct mfd_cell cell = {
+ .id = 0,
+ .name = name,
+ };
+
+ if (of_modalias_node(node, name, sizeof(name)) < 0) {
+ dev_err(dev, "modalias failure on %s\n",
+ node->full_name);
+ continue;
+ }
+ dev_dbg(dev, "adding MFD sub-device %s\n", node->name);
+ cell.of_compatible = of_get_property(node, "compatible", NULL);
+ err = mfd_add_devices(dev, ++id, &cell, 1, NULL, ec_dev->irq,
+ NULL);
+ if (err)
+ dev_err(dev, "fail to add %s\n", node->full_name);
+ }
+#endif
dev_info(dev, "Chrome EC device registered\n");
--
2.0.1
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/3] i2c: add support for Diolan DLN-2 USB-I2C adapter
From: Laurentiu Palcu
This patch adds support for the Diolan DLN-2 I2C master module. Due
to hardware limitations it does not support SMBUS quick commands.
Information about the USB protocol interface can be found in the
Programmer's Reference Manual [1], see section 6.2.2 for the I2C
master module commands and responses.
[1] https://www.diolan.com/downloads/dln-api-manual.pdf
Signed-off-by: Laurentiu Palcu
---
drivers/i2c/busses/Kconfig| 11 ++
drivers/i2c/busses/Makefile | 1 +
drivers/i2c/busses/i2c-dln2.c | 328 ++
3 files changed, 340 insertions(+)
create mode 100644 drivers/i2c/busses/i2c-dln2.c
diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index 2ac87fa..06b1e89 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -1021,4 +1021,15 @@ config SCx200_ACB
This support is also available as a module. If so, the module
will be called scx200_acb.
+config I2C_DLN2
+ tristate "Diolan DLN-2 USB I2C adapter"
+ depends on USB
+ select USB_DLN2
+ help
+ If you say yes to this option, support will be included for Diolan
+ DLN2, a USB to I2C interface.
+
+ This driver can also be built as a module. If so, the module
+ will be called i2c-dln2.
+
endmenu
diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
index 49bf07e..3118fea 100644
--- a/drivers/i2c/busses/Makefile
+++ b/drivers/i2c/busses/Makefile
@@ -100,5 +100,6 @@ obj-$(CONFIG_I2C_ELEKTOR) += i2c-elektor.o
obj-$(CONFIG_I2C_PCA_ISA) += i2c-pca-isa.o
obj-$(CONFIG_I2C_SIBYTE) += i2c-sibyte.o
obj-$(CONFIG_SCx200_ACB) += scx200_acb.o
+obj-$(CONFIG_I2C_DLN2) += i2c-dln2.o
ccflags-$(CONFIG_I2C_DEBUG_BUS) := -DDEBUG
diff --git a/drivers/i2c/busses/i2c-dln2.c b/drivers/i2c/busses/i2c-dln2.c
new file mode 100644
index 000..7befbb0
--- /dev/null
+++ b/drivers/i2c/busses/i2c-dln2.c
@@ -0,0 +1,328 @@
+/*
+ * Driver for the Diolan DLN-2 USB-I2C adapter
+ *
+ * Copyright (c) 2014 Intel Corporation
+ *
+ * Derived from:
+ * i2c-diolan-u2c.c
+ * Copyright (c) 2010-2011 Ericsson AB
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation, version 2.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define DRIVER_NAME"i2c-dln2"
+
+#define DLN2_I2C_MODULE_ID 0x03
+#define DLN2_I2C_CMD(cmd) DLN2_CMD(cmd, DLN2_I2C_MODULE_ID)
+
+/* I2C commands */
+#define CMD_I2C_GET_PORT_COUNT DLN2_I2C_CMD(0x00)
+#define CMD_I2C_ENABLE DLN2_I2C_CMD(0x01)
+#define CMD_I2C_DISABLEDLN2_I2C_CMD(0x02)
+#define CMD_I2C_IS_ENABLED DLN2_I2C_CMD(0x03)
+#define CMD_I2C_SET_FREQUENCY DLN2_I2C_CMD(0x04)
+#define CMD_I2C_GET_FREQUENCY DLN2_I2C_CMD(0x05)
+#define CMD_I2C_WRITE DLN2_I2C_CMD(0x06)
+#define CMD_I2C_READ DLN2_I2C_CMD(0x07)
+#define CMD_I2C_SCAN_DEVICES DLN2_I2C_CMD(0x08)
+#define CMD_I2C_PULLUP_ENABLE DLN2_I2C_CMD(0x09)
+#define CMD_I2C_PULLUP_DISABLE DLN2_I2C_CMD(0x0A)
+#define CMD_I2C_PULLUP_IS_ENABLED DLN2_I2C_CMD(0x0B)
+#define CMD_I2C_TRANSFER DLN2_I2C_CMD(0x0C)
+#define CMD_I2C_SET_MAX_REPLY_COUNTDLN2_I2C_CMD(0x0D)
+#define CMD_I2C_GET_MAX_REPLY_COUNTDLN2_I2C_CMD(0x0E)
+#define CMD_I2C_GET_MIN_FREQUENCY DLN2_I2C_CMD(0x40)
+#define CMD_I2C_GET_MAX_FREQUENCY DLN2_I2C_CMD(0x41)
+
+#define DLN2_I2C_FREQ_FAST 40
+#define DLN2_I2C_FREQ_STD 10
+
+#define DLN2_I2C_MAX_XFER_SIZE 256
+
+static struct dln2_mod *dln2_i2c_mod;
+
+/* Structure to hold all of our device specific stuff */
+struct dln2_i2c {
+ struct i2c_adapter adapter; /* i2c related things */
+ struct dln2_dev *dln2;
+};
+
+static uint frequency = DLN2_I2C_FREQ_STD; /* I2C clock frequency in Hz */
+
+module_param(frequency, uint, S_IRUGO | S_IWUSR);
+MODULE_PARM_DESC(frequency, "I2C clock frequency in hertz");
+
+static int dln2_i2c_set_state(struct dln2_i2c *dev, u8 state)
+{
+ int ret;
+ u8 port = 0;
+
+ ret = dln2_transfer(dev->dln2, dln2_i2c_mod,
+ state ? CMD_I2C_ENABLE : CMD_I2C_DISABLE,
+ &port, sizeof(port), NULL, NULL);
+
+ if (ret < 0)
+ return ret;
+
+ return 0;
+}
+
+#define dln2_i2c_enable(dev) dln2_i2c_set_state(dev, 1)
+#define dln2_i2c_disable(dev) dln2_i2c_set_state(dev, 0)
+
+static int dln2_i2c_set_frequency(struct dln2_i2c *dev, u32 freq)
+{
+ int ret;
+ struct tx_data {
+ u8 port;
+ __le32 freq;
+ } __packed tx;
+
+ tx.port = 0;
+ tx.freq = cpu_to_le32(freq);
+
+ ret = dln2_transfer
[PATCH 3/3] gpio: add support for the Diolan DLN-2 USB-GPIO driver
This patch adds GPIO and IRQ support for the Diolan DLN-2 GPIO module.
Information about the USB protocol interface can be found in the
Programmer's Reference Manual [1], see section 2.9 for the GPIO
module commands and responses.
[1] https://www.diolan.com/downloads/dln-api-manual.pdf
Signed-off-by: Daniel Baluta
---
drivers/gpio/Kconfig | 13 ++
drivers/gpio/Makefile| 1 +
drivers/gpio/gpio-dln2.c | 571 +++
3 files changed, 585 insertions(+)
create mode 100644 drivers/gpio/gpio-dln2.c
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 9de1515..1da9857 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -897,4 +897,17 @@ config GPIO_VIPERBOARD
River Tech's viperboard.h for detailed meaning
of the module parameters.
+config GPIO_DLN2
+ tristate "Diolan DLN2 GPIO support"
+ depends on USB
+ select USB_DLN2
+ select IRQ_DOMAIN
+
+ help
+ Select this option to enable GPIO driver for the Diolan DLN2
+ board.
+
+ This driver can also be built as a module. If so, the module
+ will be called gpio-dln2.
+
endif
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 5d024e3..eaa97a0 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -26,6 +26,7 @@ obj-$(CONFIG_GPIO_CRYSTAL_COVE) += gpio-crystalcove.o
obj-$(CONFIG_GPIO_DA9052) += gpio-da9052.o
obj-$(CONFIG_GPIO_DA9055) += gpio-da9055.o
obj-$(CONFIG_GPIO_DAVINCI) += gpio-davinci.o
+obj-$(CONFIG_GPIO_DLN2)+= gpio-dln2.o
obj-$(CONFIG_GPIO_DWAPB) += gpio-dwapb.o
obj-$(CONFIG_GPIO_EM) += gpio-em.o
obj-$(CONFIG_GPIO_EP93XX) += gpio-ep93xx.o
diff --git a/drivers/gpio/gpio-dln2.c b/drivers/gpio/gpio-dln2.c
new file mode 100644
index 000..20d2e07
--- /dev/null
+++ b/drivers/gpio/gpio-dln2.c
@@ -0,0 +1,571 @@
+/*
+ * Driver for the Diolan DLN-2 USB-GPIO adapter
+ *
+ * Copyright (c) 2014 Intel Corporation
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation, version 2.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define DRIVER_NAME "gpio-dln2"
+
+#define DLN2_GPIO_ID 0x01
+
+#define CMD_GPIO_GET_PORT_COUNTDLN2_CMD(0x00, DLN2_GPIO_ID)
+#define CMD_GPIO_GET_PIN_COUNT DLN2_CMD(0x01, DLN2_GPIO_ID)
+#define CMD_GPIO_SET_DEBOUNCE DLN2_CMD(0x04, DLN2_GPIO_ID)
+#define CMD_GPIO_GET_DEBOUNCE DLN2_CMD(0x05, DLN2_GPIO_ID)
+#define CMD_GPIO_PORT_GET_VAL DLN2_CMD(0x06, DLN2_GPIO_ID)
+#define CMD_GPIO_PIN_GET_VAL DLN2_CMD(0x0B, DLN2_GPIO_ID)
+#define CMD_GPIO_PIN_SET_OUT_VAL DLN2_CMD(0x0C, DLN2_GPIO_ID)
+#define CMD_GPIO_PIN_GET_OUT_VAL DLN2_CMD(0x0D, DLN2_GPIO_ID)
+#define CMD_GPIO_CONDITION_MET_EV DLN2_CMD(0x0F, DLN2_GPIO_ID)
+#define CMD_GPIO_PIN_ENABLEDLN2_CMD(0x10, DLN2_GPIO_ID)
+#define CMD_GPIO_PIN_DISABLE DLN2_CMD(0x11, DLN2_GPIO_ID)
+#define CMD_GPIO_PIN_SET_DIRECTION DLN2_CMD(0x13, DLN2_GPIO_ID)
+#define CMD_GPIO_PIN_GET_DIRECTION DLN2_CMD(0x14, DLN2_GPIO_ID)
+#define CMD_GPIO_PIN_SET_EVENT_CFG DLN2_CMD(0x1E, DLN2_GPIO_ID)
+#define CMD_GPIO_PIN_GET_EVENT_CFG DLN2_CMD(0x1F, DLN2_GPIO_ID)
+
+#define DLN2_GPIO_EVENT_NONE 0
+#define DLN2_GPIO_EVENT_CHANGE 1
+#define DLN2_GPIO_EVENT_LVL_HIGH 2
+#define DLN2_GPIO_EVENT_LVL_LOW3
+#define DLN2_GPIO_EVENT_CHANGE_RISING 0x11
+#define DLN2_GPIO_EVENT_CHANGE_FALLING 0x21
+#define DLN2_GPIO_EVENT_MASK 0x0F
+
+struct dln2_mod *dln2_gpio_mod, *dln2_irq_mod;
+
+#define DLN2_GPIO_MAX_PINS 32
+
+struct dln2_irq_work {
+ struct work_struct work;
+ struct dln2_gpio *dev;
+ int pin, type;
+};
+
+struct dln2_gpio {
+ struct dln2_dev *dln2;
+
+ struct gpio_chip gpio;
+
+ struct irq_domain *irq_domain;
+ DECLARE_BITMAP(irqs_masked, DLN2_GPIO_MAX_PINS);
+ DECLARE_BITMAP(irqs_enabled, DLN2_GPIO_MAX_PINS);
+ DECLARE_BITMAP(irqs_pending, DLN2_GPIO_MAX_PINS);
+ struct dln2_irq_work irq_work[DLN2_GPIO_MAX_PINS];
+};
+
+struct dln2_gpio_pin {
+ __le16 pin;
+} __packed;
+
+struct dln2_gpio_pin_val {
+ __le16 pin;
+ u8 value;
+} __packed;
+
+int dln2_gpio_get_pin_count(struct dln2_dev *dln2)
+{
+ __le16 count;
+ int ret, len = sizeof(count);
+
+ ret = dln2_transfer(dln2, dln2_gpio_mod, CMD_GPIO_GET_PIN_COUNT,
+ NULL, 0, &count, &len);
+ if (ret < 0)
+ return ret;
+
+ return le16_to_cpu(count);
+}
+
+static int dln2_gpio_pin_cmd(struct dln2_dev *dln2, int cmd, unsigned pin)
+{
+ struct dln2_gpio_pin req = {
+
[PATCH 1/3] usb: add support for Diolan DLN-2 devices
From: Octavian Purdila
This patch implements the USB part of the Diolan USB-I2C/SPI/GPIO
Master Adapter DLN-2. Details about the device can be found here:
https://www.diolan.com/i2c/i2c_interface.html.
Information about the USB protocol can be found in the Programmer's
Reference Manual [1], see section 1.7.
Because the hardware has a single transmit endpoint and a single
receive endpoint the communication between the various DLN2 drivers
and the hardware will be muxed/demuxed by this driver.
The functional DLN2 drivers (i2c, GPIO, etc.) will have to register
themselves as DLN2 modules in order to send or receive data.
Each DLN2 module will be identified by the handle field within the DLN2
message header. If a DLN2 module issues multiple commands in parallel
they will be identified by the echo counter field in the message header.
The DLN2 modules can use the dln2_transfer() function to issue a
command and wait for its response. They can also use an asynchronous
mode of operation, in which case a receive callback function is going
to be notified when messages for a specific handle are received.
Because the hardware reserves handle 0 for GPIO events, the driver
also reserves handle 0. It will be allocated to a DLN2 module only if
it is explicitly requested.
[1] https://www.diolan.com/downloads/dln-api-manual.pdf
Signed-off-by: Octavian Purdila
---
drivers/usb/misc/Kconfig | 6 +
drivers/usb/misc/Makefile | 1 +
drivers/usb/misc/dln2.c | 719 ++
include/linux/usb/dln2.h | 146 ++
4 files changed, 872 insertions(+)
create mode 100644 drivers/usb/misc/dln2.c
create mode 100644 include/linux/usb/dln2.h
diff --git a/drivers/usb/misc/Kconfig b/drivers/usb/misc/Kconfig
index 76d7720..953f521 100644
--- a/drivers/usb/misc/Kconfig
+++ b/drivers/usb/misc/Kconfig
@@ -255,3 +255,9 @@ config USB_LINK_LAYER_TEST
This driver is for generating specific traffic for Super Speed Link
Layer Test Device. Say Y only when you want to conduct USB Super Speed
Link Layer Test for host controllers.
+
+config USB_DLN2
+ tristate "Diolan DLN2 USB Driver"
+ help
+ This adds USB support for Diolan USB-I2C/SPI/GPIO
+ Master Adapter DLN-2.
diff --git a/drivers/usb/misc/Makefile b/drivers/usb/misc/Makefile
index 65b0402..767264e 100644
--- a/drivers/usb/misc/Makefile
+++ b/drivers/usb/misc/Makefile
@@ -25,6 +25,7 @@ obj-$(CONFIG_USB_USS720) += uss720.o
obj-$(CONFIG_USB_SEVSEG) += usbsevseg.o
obj-$(CONFIG_USB_YUREX)+= yurex.o
obj-$(CONFIG_USB_HSIC_USB3503) += usb3503.o
+obj-$(CONFIG_USB_DLN2) += dln2.o
obj-$(CONFIG_USB_SISUSBVGA)+= sisusbvga/
obj-$(CONFIG_USB_LINK_LAYER_TEST) += lvstest.o
diff --git a/drivers/usb/misc/dln2.c b/drivers/usb/misc/dln2.c
new file mode 100644
index 000..5bfa850
--- /dev/null
+++ b/drivers/usb/misc/dln2.c
@@ -0,0 +1,719 @@
+/*
+ * Driver for the Diolan DLN-2 USB adapter
+ *
+ * Copyright (c) 2014 Intel Corporation
+ *
+ * Derived from:
+ * i2c-diolan-u2c.c
+ * Copyright (c) 2010-2011 Ericsson AB
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation, version 2.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define DRIVER_NAME"usb-dln2"
+
+#define DLN2_GENERIC_MODULE_ID 0x00
+#define DLN2_GENERIC_CMD(cmd) DLN2_CMD(cmd, DLN2_GENERIC_MODULE_ID)
+
+/* generic commands */
+#define CMD_GET_DEVICE_VER DLN2_GENERIC_CMD(0x30)
+#define CMD_GET_DEVICE_SN DLN2_GENERIC_CMD(0x31)
+
+#define DLN2_HW_ID 0x200
+
+#define DLN2_USB_TIMEOUT 100 /* in ms */
+
+#define DLN2_MAX_RX_SLOTS 16
+
+#include
+
+struct dln2_rx_context {
+ struct completion done;
+ struct urb *urb;
+ bool connected;
+};
+
+struct dln2_rx_slots {
+ /* RX slots bitmap */
+ DECLARE_BITMAP(bmap, DLN2_MAX_RX_SLOTS);
+
+ /* used to wait for a free RX slot */
+ wait_queue_head_t wq;
+
+ /* used to wait for an RX operation to complete */
+ struct dln2_rx_context slots[DLN2_MAX_RX_SLOTS];
+
+ /* avoid races between free_rx_slot and dln2_transfer_rx_cb */
+ spinlock_t lock;
+};
+
+static int find_free_slot(struct dln2_rx_slots *rxs, int *slot)
+{
+ unsigned long flags;
+
+ spin_lock_irqsave(&rxs->lock, flags);
+
+ *slot = bitmap_find_free_region(rxs->bmap, DLN2_MAX_RX_SLOTS, 0) - 1;
+
+ if (*slot >= 0) {
+ struct dln2_rx_context *rxc = &rxs->slots[*slot];
+
+ init_completion(&rxc->done);
+ rxc->connected = true;
+ }
+
+ spin_unlock_irqrestore(&rxs->lock, flags);
+
+ return *slot >= 0;
+}
+
+static int alloc_rx_slot
[PATCH 0/3] dln-2: Add support for Diolan DLN-2 devices
This patch series adds support for Diolan USB-I2C/GPIO Master Adapter DLN-2. Details about device can be found here: https://www.diolan.com/i2c/i2c_interface.html. There is no support for SPI part yet. Daniel Baluta (1): gpio: add support for the Diolan DLN-2 USB-GPIO driver Laurentiu Palcu (1): i2c: add support for Diolan DLN-2 USB-I2C adapter Octavian Purdila (1): usb: add support for Diolan DLN-2 devices drivers/gpio/Kconfig | 13 + drivers/gpio/Makefile | 1 + drivers/gpio/gpio-dln2.c | 571 + drivers/i2c/busses/Kconfig| 11 + drivers/i2c/busses/Makefile | 1 + drivers/i2c/busses/i2c-dln2.c | 328 +++ drivers/usb/misc/Kconfig | 6 + drivers/usb/misc/Makefile | 1 + drivers/usb/misc/dln2.c | 719 ++ include/linux/usb/dln2.h | 146 + 10 files changed, 1797 insertions(+) create mode 100644 drivers/gpio/gpio-dln2.c create mode 100644 drivers/i2c/busses/i2c-dln2.c create mode 100644 drivers/usb/misc/dln2.c create mode 100644 include/linux/usb/dln2.h -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] i2c: designware: deduce speed mode from device tree setting
Hi,
With the patch "i2c designware add support of I2C standard mode" I already
proposed:
- I2C standard mode is selected with 100kHz clock frequency.
- I2C fast mode is selected with 400kHy clock frequency.
- EINVAL error is returned if clock frequency is not 10 and not 40.
but this patch seems not available yet.
What about the other patch "i2c designware make SCL and SDA falling time
configurable" ?
In i2c-designware_platdrv.c the flag DW_IC_CON_SPEED_xxx is well set
depending on the mode:
if (clk_freq == 10)
dev->master_cfg = DW_IC_CON_MASTER | DW_IC_CON_SLAVE_DISABLE |
DW_IC_CON_RESTART_EN | DW_IC_CON_SPEED_STD;
else
dev->master_cfg = DW_IC_CON_MASTER | DW_IC_CON_SLAVE_DISABLE |
DW_IC_CON_RESTART_EN | DW_IC_CON_SPEED_FAST;
So for me everything should be fine if there patches are applied.
Regards,
Romain
- Original Message -
From: "Mark Rutland"
To: at...@opensource.altera.com
Cc: w...@the-dreams.de, bar...@tkos.co.il, "mika westerberg"
, "grant likely" ,
robh...@kernel.org, skuri...@pobox.com, "Romain Baeriswyl"
, "rafael j wysocki" ,
a...@linux.intel.com, linux-i2c@vger.kernel.org, linux-ker...@vger.kernel.org,
devicet...@vger.kernel.org, "delicious quinoa" ,
dingu...@opensource.altera.com, yvand...@opensource.altera.com
Sent: Wednesday, August 20, 2014 11:22:57 AM
Subject: Re: [PATCH] i2c: designware: deduce speed mode from device tree setting
On Tue, Aug 19, 2014 at 09:18:49PM +0100, at...@opensource.altera.com wrote:
> From: Alan Tull
>
> Use the documented, but unimplemented "clock-frequency"
> Device Tree setting as a guide on whether to set the speed
> mode bits in DW_IC_CON to standard or fast i2c mode.
>
> Previously, the driver was hardwired to fast mode. Default
> to fast mode if the "clock-frequency" property is not present
> for backwards compatiblity.
>
> Signed-off-by: Alan Tull
> ---
> drivers/i2c/busses/i2c-designware-platdrv.c | 10 --
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c
> b/drivers/i2c/busses/i2c-designware-platdrv.c
> index bc87733..18cd3d9 100644
> --- a/drivers/i2c/busses/i2c-designware-platdrv.c
> +++ b/drivers/i2c/busses/i2c-designware-platdrv.c
> @@ -122,7 +122,8 @@ static int dw_i2c_probe(struct platform_device *pdev)
> struct dw_i2c_dev *dev;
> struct i2c_adapter *adap;
> struct resource *mem;
> - int irq, r;
> + int irq, r, ret, speed = DW_IC_CON_SPEED_FAST;
> + u32 bus_rate;
>
> irq = platform_get_irq(pdev, 0);
> if (irq < 0) {
> @@ -167,6 +168,11 @@ static int dw_i2c_probe(struct platform_device *pdev)
> of_property_read_u32(pdev->dev.of_node,
>"i2c-scl-falling-time-ns",
>&dev->scl_falling_time);
> +
> + ret = of_property_read_u32(pdev->dev.of_node,
> +"clock-frequency", &bus_rate);
> + if (!ret && (bus_rate <= 10))
> + speed = DW_IC_CON_SPEED_STD;
This looks a bit odd.
If the device only supports two particular speeds why do we accept any
other speed in the clock-frequency property? Surely we should at least
warn that something was off?
Thanks,
Mark
> }
>
> dev->functionality =
> @@ -177,7 +183,7 @@ static int dw_i2c_probe(struct platform_device *pdev)
> I2C_FUNC_SMBUS_WORD_DATA |
> I2C_FUNC_SMBUS_I2C_BLOCK;
> dev->master_cfg = DW_IC_CON_MASTER | DW_IC_CON_SLAVE_DISABLE |
> - DW_IC_CON_RESTART_EN | DW_IC_CON_SPEED_FAST;
> + DW_IC_CON_RESTART_EN | speed;
>
> /* Try first if we can configure the device from ACPI */
> r = dw_i2c_acpi_configure(pdev);
> --
> 1.7.9.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] I2C/ACPI: Fix possible ZERO_SIZE_PTR pointer dereferencing error.
On Wed, Aug 20, 2014 at 04:59:59PM +0800, Lan Tianyu wrote: > On 08/19/2014 11:48 PM, Mika Westerberg wrote: > >On Tue, Aug 19, 2014 at 10:38:08AM -0500, Wolfram Sang wrote: > >>On Tue, Aug 19, 2014 at 06:16:49PM +0300, Mika Westerberg wrote: > >>>On Tue, Aug 19, 2014 at 10:03:55AM -0500, Wolfram Sang wrote: > On Tue, Aug 12, 2014 at 10:33:38AM +0800, Xiubo Li wrote: > >Since we cannot make sure the 'data_len' will always be none zero > >here, and then if 'data_len' equals to zero, the kzalloc() will > >return ZERO_SIZE_PTR, which equals to ((void *)16). > > I assume the read request with length == 0 comes from a broken BIOS? > >>> > >>>I'm also interested. Does this trigger in a real system? > >> > >>Even if not now, we should consider potentially broken BIOSes, or? Which > >>extends the question to: Do we need even more sanity checks when taking > >>broken BIOSes into account? > > > >Typically ACPICA has done this work for us (e.g it fixes things upfront so > >that we get sane data). I'm not sure if it does that for I2C Operation > >Regions, though (that's why I'm asking if it happens in a real system or is > >this more like a theoretical possibility). > > > >Tianyu, any comments? > > > > Sorry for later response due to leave home today. acpi_gsb_i2c_read_bytes() > dedicates for GenericSerialBus Read/Write N Bytes protocol(ACPI Spec > 5.5.2.4.5.3.8). Bios wants to read N Bytes when uses this protocol and the > length specified by Bios should be greater than 1. If the Bios specified 0 > bytes, the associated function(E,G read battery info) would be totally > unusable. > I think such Bios can't pass through Windows certification:). From this > point, I > think the check is not necessary. > > If you still thought this maybe happen, I think it makes more sense to add the > check length in the ACPICA. Because ACPICA will allocate a data buffer for I2C > ACPI operation region access before call the callback. The buffer length will > be > result of protocol head length plus data length. If data length is 0 and this > means the access will be invalid and ACPICA should ignore it or produce a > warning. > Thanks Tianyu for the clarification. So Wolfram, up to you -- in principle this check is not needed but it doesn't do any harm either. -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] i2c: designware: device tree bindings for i2c speed mode
On Tue, Aug 19, 2014 at 07:34:35PM +0100, at...@opensource.altera.com wrote:
> From: Alan Tull
>
> Add "speed-mode" Device Tree property to select between
> standard and fast i2c mode. Previously, driver was hardwired
> as fast mode. Default to fast mode if property is not
> present.
>
> Signed-off-by: Alan Tull
> ---
> .../devicetree/bindings/i2c/i2c-designware.txt |3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/i2c/i2c-designware.txt
> b/Documentation/devicetree/bindings/i2c/i2c-designware.txt
> index 5199b0c..0e4cd21 100644
> --- a/Documentation/devicetree/bindings/i2c/i2c-designware.txt
> +++ b/Documentation/devicetree/bindings/i2c/i2c-designware.txt
> @@ -9,6 +9,8 @@ Required properties :
> Recommended properties :
>
> - clock-frequency : desired I2C bus clock frequency in Hz.
> + - speed-mode : 0 = standard (0 - 100Kb/s)
> + : 1 = fast (<= 400Kb/s) <== default
This is a bit opaque.
Is this a limit on the max bit-rate the device should operate at?
Why not just have an optional boolean property to limit to standard
speed?
Thanks,
Mark.
>
> Optional properties :
> - i2c-sda-hold-time-ns : should contain the SDA hold time in nanoseconds.
> @@ -29,6 +31,7 @@ Example :
> reg = <0xf 0x1000>;
> interrupts = <11>;
> clock-frequency = <40>;
> + speed-mode = <0>;
> };
>
> i2c@112 {
> --
> 1.7.9.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] i2c: designware: device tree bindings for i2c speed mode
On Wed, Aug 20, 2014 at 10:34:59AM +0100, Mark Rutland wrote: > On Tue, Aug 19, 2014 at 07:34:35PM +0100, at...@opensource.altera.com wrote: > > From: Alan Tull > > > > Add "speed-mode" Device Tree property to select between > > standard and fast i2c mode. Previously, driver was hardwired > > as fast mode. Default to fast mode if property is not > > present. > > > > Signed-off-by: Alan Tull > > --- > > .../devicetree/bindings/i2c/i2c-designware.txt |3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/i2c/i2c-designware.txt > > b/Documentation/devicetree/bindings/i2c/i2c-designware.txt > > index 5199b0c..0e4cd21 100644 > > --- a/Documentation/devicetree/bindings/i2c/i2c-designware.txt > > +++ b/Documentation/devicetree/bindings/i2c/i2c-designware.txt > > @@ -9,6 +9,8 @@ Required properties : > > Recommended properties : > > > > - clock-frequency : desired I2C bus clock frequency in Hz. > > + - speed-mode : 0 = standard (0 - 100Kb/s) > > + : 1 = fast (<= 400Kb/s) <== default > > This is a bit opaque. > > Is this a limit on the max bit-rate the device should operate at? > > Why not just have an optional boolean property to limit to standard > speed? I see there's a new patch doing this based on clock-frequency, so feel free to ignore this. Mark. -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] i2c: designware: deduce speed mode from device tree setting
On Tue, Aug 19, 2014 at 09:18:49PM +0100, at...@opensource.altera.com wrote:
> From: Alan Tull
>
> Use the documented, but unimplemented "clock-frequency"
> Device Tree setting as a guide on whether to set the speed
> mode bits in DW_IC_CON to standard or fast i2c mode.
>
> Previously, the driver was hardwired to fast mode. Default
> to fast mode if the "clock-frequency" property is not present
> for backwards compatiblity.
>
> Signed-off-by: Alan Tull
> ---
> drivers/i2c/busses/i2c-designware-platdrv.c | 10 --
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c
> b/drivers/i2c/busses/i2c-designware-platdrv.c
> index bc87733..18cd3d9 100644
> --- a/drivers/i2c/busses/i2c-designware-platdrv.c
> +++ b/drivers/i2c/busses/i2c-designware-platdrv.c
> @@ -122,7 +122,8 @@ static int dw_i2c_probe(struct platform_device *pdev)
> struct dw_i2c_dev *dev;
> struct i2c_adapter *adap;
> struct resource *mem;
> - int irq, r;
> + int irq, r, ret, speed = DW_IC_CON_SPEED_FAST;
> + u32 bus_rate;
>
> irq = platform_get_irq(pdev, 0);
> if (irq < 0) {
> @@ -167,6 +168,11 @@ static int dw_i2c_probe(struct platform_device *pdev)
> of_property_read_u32(pdev->dev.of_node,
>"i2c-scl-falling-time-ns",
>&dev->scl_falling_time);
> +
> + ret = of_property_read_u32(pdev->dev.of_node,
> +"clock-frequency", &bus_rate);
> + if (!ret && (bus_rate <= 10))
> + speed = DW_IC_CON_SPEED_STD;
This looks a bit odd.
If the device only supports two particular speeds why do we accept any
other speed in the clock-frequency property? Surely we should at least
warn that something was off?
Thanks,
Mark
> }
>
> dev->functionality =
> @@ -177,7 +183,7 @@ static int dw_i2c_probe(struct platform_device *pdev)
> I2C_FUNC_SMBUS_WORD_DATA |
> I2C_FUNC_SMBUS_I2C_BLOCK;
> dev->master_cfg = DW_IC_CON_MASTER | DW_IC_CON_SLAVE_DISABLE |
> - DW_IC_CON_RESTART_EN | DW_IC_CON_SPEED_FAST;
> + DW_IC_CON_RESTART_EN | speed;
>
> /* Try first if we can configure the device from ACPI */
> r = dw_i2c_acpi_configure(pdev);
> --
> 1.7.9.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] I2C/ACPI: Fix possible ZERO_SIZE_PTR pointer dereferencing error.
On 08/19/2014 11:48 PM, Mika Westerberg wrote: On Tue, Aug 19, 2014 at 10:38:08AM -0500, Wolfram Sang wrote: On Tue, Aug 19, 2014 at 06:16:49PM +0300, Mika Westerberg wrote: On Tue, Aug 19, 2014 at 10:03:55AM -0500, Wolfram Sang wrote: On Tue, Aug 12, 2014 at 10:33:38AM +0800, Xiubo Li wrote: Since we cannot make sure the 'data_len' will always be none zero here, and then if 'data_len' equals to zero, the kzalloc() will return ZERO_SIZE_PTR, which equals to ((void *)16). I assume the read request with length == 0 comes from a broken BIOS? I'm also interested. Does this trigger in a real system? Even if not now, we should consider potentially broken BIOSes, or? Which extends the question to: Do we need even more sanity checks when taking broken BIOSes into account? Typically ACPICA has done this work for us (e.g it fixes things upfront so that we get sane data). I'm not sure if it does that for I2C Operation Regions, though (that's why I'm asking if it happens in a real system or is this more like a theoretical possibility). Tianyu, any comments? Sorry for later response due to leave home today. acpi_gsb_i2c_read_bytes() dedicates for GenericSerialBus Read/Write N Bytes protocol(ACPI Spec 5.5.2.4.5.3.8). Bios wants to read N Bytes when uses this protocol and the length specified by Bios should be greater than 1. If the Bios specified 0 bytes, the associated function(E,G read battery info) would be totally unusable. I think such Bios can't pass through Windows certification:). From this point, I think the check is not necessary. If you still thought this maybe happen, I think it makes more sense to add the check length in the ACPICA. Because ACPICA will allocate a data buffer for I2C ACPI operation region access before call the callback. The buffer length will be result of protocol head length plus data length. If data length is 0 and this means the access will be invalid and ACPICA should ignore it or produce a warning. -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] I2C/ACPI: Fix possible ZERO_SIZE_PTR pointer dereferencing error.
On Wed, Aug 20, 2014 at 02:37:57AM +, li.xi...@freescale.com wrote: > > On Tue, Aug 19, 2014 at 10:38:08AM -0500, Wolfram Sang wrote: > > > On Tue, Aug 19, 2014 at 06:16:49PM +0300, Mika Westerberg wrote: > > > > On Tue, Aug 19, 2014 at 10:03:55AM -0500, Wolfram Sang wrote: > > > > > On Tue, Aug 12, 2014 at 10:33:38AM +0800, Xiubo Li wrote: > > > > > > Since we cannot make sure the 'data_len' will always be none zero > > > > > > here, > > > > > > and then if 'data_len' equals to zero, the kzalloc() will return > > ZERO_SIZE_PTR, > > > > > > which equals to ((void *)16). > > > > > > > > > > I assume the read request with length == 0 comes from a broken BIOS? > > > > > > > > I'm also interested. Does this trigger in a real system? > > > > > > Even if not now, we should consider potentially broken BIOSes, or? Which > > > extends the question to: Do we need even more sanity checks when taking > > > broken BIOSes into account? > > > > Typically ACPICA has done this work for us (e.g it fixes things upfront > > so that we get sane data). I'm not sure if it does that for I2C > > Operation Regions, though (that's why I'm asking if it happens in a real > > system or is this more like a theoretical possibility). > > > Yes, it a theoretical potentially risk here for me, but not sure in the > future. > I have encountered many issues of this risk for different codes, which can be > reproduced very easily mostly. OK, so it didn̈́'t happen (yet :-)) in a real system. I would like to check with Tianyu or Lv (Cc'd) whether ACPICA protects us from this kind of insanity and if it doesn't right now, it probably should. I'm fine with the patch itself to be merged. -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] i2c: designware: deduce speed mode from device tree setting
On Tue, Aug 19, 2014 at 03:18:49PM -0500, at...@opensource.altera.com wrote: > From: Alan Tull > > Use the documented, but unimplemented "clock-frequency" > Device Tree setting as a guide on whether to set the speed > mode bits in DW_IC_CON to standard or fast i2c mode. > > Previously, the driver was hardwired to fast mode. Default > to fast mode if the "clock-frequency" property is not present > for backwards compatiblity. > > Signed-off-by: Alan Tull I don't know much about the Device Tree bindings but the patch looks good to me, Acked-by: Mika Westerberg -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
