| Message ID | 20200331105051.58896-44-jiada_wang@mentor.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | atmel_mxt_ts misc | expand |
31.03.2020 13:50, Jiada Wang пишет: > Add support to set max bytes transferred in an i2c transaction > > Signed-off-by: Jiada Wang <jiada_wang@mentor.com> > --- > Documentation/devicetree/bindings/input/atmel,maxtouch.txt | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/Documentation/devicetree/bindings/input/atmel,maxtouch.txt b/Documentation/devicetree/bindings/input/atmel,maxtouch.txt > index 126266891cdc..37a798cab0dd 100644 > --- a/Documentation/devicetree/bindings/input/atmel,maxtouch.txt > +++ b/Documentation/devicetree/bindings/input/atmel,maxtouch.txt > @@ -45,6 +45,8 @@ Optional properties for main touchpad device: > > - atmel,input_name: Override name of input device from the default. > > +- atmel,mtu: Maximum number of bytes that can read/transferred in an i2c transaction > + > Example: > > touch@4b { > @@ -52,4 +54,5 @@ Example: > reg = <0x4b>; > interrupt-parent = <&gpio>; > interrupts = <TEGRA_GPIO(W, 3) IRQ_TYPE_LEVEL_LOW>; > + atmel,mtu = <200> > }; > Is this a software (firmware) limitation which varies from version to version? If the limitation is per-hardware version, then perhaps should be better to have a hardcoded per-hardware table of limits in the driver?
Hi Dmitry On 2020/04/01 23:38, Dmitry Osipenko wrote: > 31.03.2020 13:50, Jiada Wang пишет: >> Add support to set max bytes transferred in an i2c transaction >> >> Signed-off-by: Jiada Wang <jiada_wang@mentor.com> >> --- >> Documentation/devicetree/bindings/input/atmel,maxtouch.txt | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/Documentation/devicetree/bindings/input/atmel,maxtouch.txt b/Documentation/devicetree/bindings/input/atmel,maxtouch.txt >> index 126266891cdc..37a798cab0dd 100644 >> --- a/Documentation/devicetree/bindings/input/atmel,maxtouch.txt >> +++ b/Documentation/devicetree/bindings/input/atmel,maxtouch.txt >> @@ -45,6 +45,8 @@ Optional properties for main touchpad device: >> >> - atmel,input_name: Override name of input device from the default. >> >> +- atmel,mtu: Maximum number of bytes that can read/transferred in an i2c transaction >> + >> Example: >> >> touch@4b { >> @@ -52,4 +54,5 @@ Example: >> reg = <0x4b>; >> interrupt-parent = <&gpio>; >> interrupts = <TEGRA_GPIO(W, 3) IRQ_TYPE_LEVEL_LOW>; >> + atmel,mtu = <200> >> }; >> > > Is this a software (firmware) limitation which varies from version to > version? > the timeout issue trying to be addressed in this patch is from software, one of our board a Serializer/Deserializer bridge exists between the SoC (imx6) and the Atmel touch controller. imx6 i2c controller driver has a timeout value(100ms) for each i2c transaction, Large i2c read transaction failed to complete within this timeout value and therefore imx6 i2c controller driver aborts the transaction and returns failure. Therefore this patch was created to split the large i2c transaction into smaller chunks which can complete within the timeout defined by i2c controller driver. Thanks, Jiada > If the limitation is per-hardware version, then perhaps should be better > to have a hardcoded per-hardware table of limits in the driver? >
07.04.2020 12:27, Wang, Jiada пишет: .. >> Is this a software (firmware) limitation which varies from version to >> version? >> > > the timeout issue trying to be addressed in this patch is from software, > one of our board a Serializer/Deserializer bridge exists between the SoC > (imx6) and the Atmel touch controller. > imx6 i2c controller driver has a timeout value(100ms) for each i2c > transaction, > Large i2c read transaction failed to complete within this timeout value > and therefore imx6 i2c controller driver aborts the transaction > and returns failure. > > Therefore this patch was created to split the large i2c transaction into > smaller chunks which can complete > within the timeout defined by i2c controller driver. Isn't it possible to use the max_read/write_len of the generic struct i2c_adapter_quirks for limiting the transfer size? BTW, it looks like the i.MX I2C driver doesn't specify the i2c_adapter_quirks, which probably needs to be fixed.
Hi Dmitry On 2020/04/07 23:47, Dmitry Osipenko wrote: > 07.04.2020 12:27, Wang, Jiada пишет: > .. >>> Is this a software (firmware) limitation which varies from version to >>> version? >>> >> >> the timeout issue trying to be addressed in this patch is from software, >> one of our board a Serializer/Deserializer bridge exists between the SoC >> (imx6) and the Atmel touch controller. >> imx6 i2c controller driver has a timeout value(100ms) for each i2c >> transaction, >> Large i2c read transaction failed to complete within this timeout value >> and therefore imx6 i2c controller driver aborts the transaction >> and returns failure. >> >> Therefore this patch was created to split the large i2c transaction into >> smaller chunks which can complete >> within the timeout defined by i2c controller driver. > > Isn't it possible to use the max_read/write_len of the generic struct > i2c_adapter_quirks for limiting the transfer size? > > BTW, it looks like the i.MX I2C driver doesn't specify the > i2c_adapter_quirks, which probably needs to be fixed. > yes, i.MX I2C driver can specify i2c_adapter_quirks to limit the size be transferred in one transaction. But even in this case, mxt_process_messages_t44() fails when it tries to transfer data count larger than max_read/write_len set in i.MX I2C driver, which we would like to avoid. Thanks, Jiada
09.04.2020 09:25, Wang, Jiada пишет: > Hi Dmitry > > On 2020/04/07 23:47, Dmitry Osipenko wrote: >> 07.04.2020 12:27, Wang, Jiada пишет: >> .. >>>> Is this a software (firmware) limitation which varies from version to >>>> version? >>>> >>> >>> the timeout issue trying to be addressed in this patch is from software, >>> one of our board a Serializer/Deserializer bridge exists between the SoC >>> (imx6) and the Atmel touch controller. >>> imx6 i2c controller driver has a timeout value(100ms) for each i2c >>> transaction, >>> Large i2c read transaction failed to complete within this timeout value >>> and therefore imx6 i2c controller driver aborts the transaction >>> and returns failure. >>> >>> Therefore this patch was created to split the large i2c transaction into >>> smaller chunks which can complete >>> within the timeout defined by i2c controller driver. >> >> Isn't it possible to use the max_read/write_len of the generic struct >> i2c_adapter_quirks for limiting the transfer size? >> >> BTW, it looks like the i.MX I2C driver doesn't specify the >> i2c_adapter_quirks, which probably needs to be fixed. >> > yes, i.MX I2C driver can specify i2c_adapter_quirks to limit the size be > transferred in one transaction. > > But even in this case, mxt_process_messages_t44() fails when it tries to > transfer data count larger than max_read/write_len set in i.MX I2C > driver, which we would like to avoid. IIUC, the transfer's limitation is a part of I2C controller hardware and not the touch controller, so it should be wrong to describe that limitation in the maxtouch's DT node. I meant that we probably could set the data->mtu based on i2c_client->adapter->quirks->max_read and then the DT property shouldn't be needed, couldn't this be done? The I2C core only rejects transfers that don't fit into the max_read/write_len and nothing more.
Hi Dmitry On 2020/04/10 0:10, Dmitry Osipenko wrote: > 09.04.2020 09:25, Wang, Jiada пишет: >> Hi Dmitry >> >> On 2020/04/07 23:47, Dmitry Osipenko wrote: >>> 07.04.2020 12:27, Wang, Jiada пишет: >>> .. >>>>> Is this a software (firmware) limitation which varies from version to >>>>> version? >>>>> >>>> >>>> the timeout issue trying to be addressed in this patch is from software, >>>> one of our board a Serializer/Deserializer bridge exists between the SoC >>>> (imx6) and the Atmel touch controller. >>>> imx6 i2c controller driver has a timeout value(100ms) for each i2c >>>> transaction, >>>> Large i2c read transaction failed to complete within this timeout value >>>> and therefore imx6 i2c controller driver aborts the transaction >>>> and returns failure. >>>> >>>> Therefore this patch was created to split the large i2c transaction into >>>> smaller chunks which can complete >>>> within the timeout defined by i2c controller driver. >>> >>> Isn't it possible to use the max_read/write_len of the generic struct >>> i2c_adapter_quirks for limiting the transfer size? >>> >>> BTW, it looks like the i.MX I2C driver doesn't specify the >>> i2c_adapter_quirks, which probably needs to be fixed. >>> >> yes, i.MX I2C driver can specify i2c_adapter_quirks to limit the size be >> transferred in one transaction. >> >> But even in this case, mxt_process_messages_t44() fails when it tries to >> transfer data count larger than max_read/write_len set in i.MX I2C >> driver, which we would like to avoid. > > IIUC, the transfer's limitation is a part of I2C controller hardware and > not the touch controller, so it should be wrong to describe that > limitation in the maxtouch's DT node. > > I meant that we probably could set the data->mtu based on > i2c_client->adapter->quirks->max_read and then the DT property shouldn't > be needed, couldn't this be done? >Thanks, now I understand your point, and yes, by this way, we can address the I2C controller limitation issue by its own configuration. I will replace this commit with your proposed solution Thanks, jiada > The I2C core only rejects transfers that don't fit into the > max_read/write_len and nothing more. >
diff --git a/Documentation/devicetree/bindings/input/atmel,maxtouch.txt b/Documentation/devicetree/bindings/input/atmel,maxtouch.txt index 126266891cdc..37a798cab0dd 100644 --- a/Documentation/devicetree/bindings/input/atmel,maxtouch.txt +++ b/Documentation/devicetree/bindings/input/atmel,maxtouch.txt @@ -45,6 +45,8 @@ Optional properties for main touchpad device: - atmel,input_name: Override name of input device from the default. +- atmel,mtu: Maximum number of bytes that can read/transferred in an i2c transaction + Example: touch@4b { @@ -52,4 +54,5 @@ Example: reg = <0x4b>; interrupt-parent = <&gpio>; interrupts = <TEGRA_GPIO(W, 3) IRQ_TYPE_LEVEL_LOW>; + atmel,mtu = <200> };
Add support to set max bytes transferred in an i2c transaction Signed-off-by: Jiada Wang <jiada_wang@mentor.com> --- Documentation/devicetree/bindings/input/atmel,maxtouch.txt | 3 +++ 1 file changed, 3 insertions(+)