* [PATCH v4 0/3] i2c: gpio: support write-only sda
@ 2023-01-17 20:34 Heiner Kallweit
2023-01-17 20:36 ` [PATCH v4 1/3] dt-bindings: i2c-gpio: Add properties for dealing with write-only SDA/SCL w/o pullup Heiner Kallweit
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Heiner Kallweit @ 2023-01-17 20:34 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Wolfram Sang, Peter Rosin
Cc: linux-i2c, devicetree
There are slave devices that understand I2C but have read-only SDA and
SCL. Examples are FD650 7-segment LED controller and its derivatives.
Typical board designs don't even have a pull-up for both pins.
Therefore add properties for not using open-drain. For write-only SCL
we have a property already, add one for write-only SDA.
v2:
- improve commit message for patch 1
v3:
- patch 2: check for adap->getsda in readbytes()
- patch 2: align warning message level for info on missing getscl/getsda
- patch 3: improve description of attribute sda_is_output_only
v4:
- patch 1: add no-pullup properties
- patch 2: handle SDA and SCL independently
- patch 2: properly handle case that SDA is NULL but SCL not
- patch 3: handle new no-pullup attributes
Heiner Kallweit (3):
dt-bindings: i2c-gpio: Add properties for dealing with write-only SDA/SCL w/o pullup
i2c: algo: bit: allow getsda to be NULL
i2c: gpio: support write-only sda/scl w/o pull-up
.../devicetree/bindings/i2c/i2c-gpio.yaml | 16 ++++
drivers/i2c/algos/i2c-algo-bit.c | 77 +++++++++----------
drivers/i2c/busses/i2c-gpio.c | 13 +++-
include/linux/platform_data/i2c-gpio.h | 9 +++
4 files changed, 70 insertions(+), 45 deletions(-)
--
2.39.0
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v4 1/3] dt-bindings: i2c-gpio: Add properties for dealing with write-only SDA/SCL w/o pullup
2023-01-17 20:34 [PATCH v4 0/3] i2c: gpio: support write-only sda Heiner Kallweit
@ 2023-01-17 20:36 ` Heiner Kallweit
2023-01-18 16:32 ` Rob Herring
2023-01-17 20:38 ` [PATCH v4 2/3] i2c: algo: bit: allow getsda to be NULL Heiner Kallweit
2023-01-17 20:39 ` [PATCH v4 3/3] i2c: gpio: support write-only sda/scl w/o pull-up Heiner Kallweit
2 siblings, 1 reply; 6+ messages in thread
From: Heiner Kallweit @ 2023-01-17 20:36 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Wolfram Sang, Peter Rosin
Cc: linux-i2c, devicetree
There are slave devices that understand I2C but have read-only SDA and
SCL. Examples are FD650 7-segment LED controller and its derivatives.
Typical board designs don't even have a pull-up for both pins.
Therefore add properties for not using open-drain. For write-only SCL
we have a property already, add one for write-only SDA.
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
v4:
- add no-pullup properties
---
.../devicetree/bindings/i2c/i2c-gpio.yaml | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/Documentation/devicetree/bindings/i2c/i2c-gpio.yaml b/Documentation/devicetree/bindings/i2c/i2c-gpio.yaml
index e0d76d5eb..67898cc52 100644
--- a/Documentation/devicetree/bindings/i2c/i2c-gpio.yaml
+++ b/Documentation/devicetree/bindings/i2c/i2c-gpio.yaml
@@ -33,6 +33,10 @@ properties:
open drain.
maxItems: 1
+ i2c-gpio,sda-output-only:
+ description: sda as output only
+ type: boolean
+
i2c-gpio,scl-output-only:
description: scl as output only
type: boolean
@@ -63,6 +67,18 @@ properties:
GPIO line used for SCL into open drain mode, and that something is not
the GPIO chip. It is essentially an inconsistency flag.
+ i2c-gpio,sda-has-no-pullup:
+ type: boolean
+ description: sda is used in a non-compliant way and has no pull-up.
+ Therefore disable open-drain. This property is mutually-exclusive
+ with i2c-gpio,sda-open-drain.
+
+ i2c-gpio,scl-has-no-pullup:
+ type: boolean
+ description: scl is used in a non-compliant way and has no pull-up.
+ Therefore disable open-drain. This property is mutually-exclusive
+ with i2c-gpio,scl-open-drain.
+
required:
- compatible
- sda-gpios
--
2.39.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v4 2/3] i2c: algo: bit: allow getsda to be NULL
2023-01-17 20:34 [PATCH v4 0/3] i2c: gpio: support write-only sda Heiner Kallweit
2023-01-17 20:36 ` [PATCH v4 1/3] dt-bindings: i2c-gpio: Add properties for dealing with write-only SDA/SCL w/o pullup Heiner Kallweit
@ 2023-01-17 20:38 ` Heiner Kallweit
2023-01-17 20:39 ` [PATCH v4 3/3] i2c: gpio: support write-only sda/scl w/o pull-up Heiner Kallweit
2 siblings, 0 replies; 6+ messages in thread
From: Heiner Kallweit @ 2023-01-17 20:38 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Wolfram Sang, Peter Rosin
Cc: linux-i2c, devicetree
This is in preparation of supporting write-only SDA in i2c-gpio.
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
v3:
- check for adap->getsda in readbytes()
- align warning message level for info on missing getscl/getsda
v4:
- handle SDA and SCL independently
- properly handle case that SDA is NULL but SCL not
---
drivers/i2c/algos/i2c-algo-bit.c | 77 +++++++++++++++-----------------
1 file changed, 35 insertions(+), 42 deletions(-)
diff --git a/drivers/i2c/algos/i2c-algo-bit.c b/drivers/i2c/algos/i2c-algo-bit.c
index fc90293af..eddf25b90 100644
--- a/drivers/i2c/algos/i2c-algo-bit.c
+++ b/drivers/i2c/algos/i2c-algo-bit.c
@@ -184,8 +184,9 @@ static int i2c_outb(struct i2c_adapter *i2c_adap, unsigned char c)
/* read ack: SDA should be pulled down by slave, or it may
* NAK (usually to report problems with the data we wrote).
+ * Always report ACK if SDA is write-only.
*/
- ack = !getsda(adap); /* ack: sda is pulled low -> success */
+ ack = !adap->getsda || !getsda(adap); /* ack: sda is pulled low -> success */
bit_dbg(2, &i2c_adap->dev, "i2c_outb: 0x%02x %s\n", (int)c,
ack ? "A" : "NA");
@@ -238,71 +239,55 @@ static int test_bus(struct i2c_adapter *i2c_adap)
return -ENODEV;
}
+ if (adap->getsda == NULL)
+ pr_info("%s: SDA is write-only, testing not possible\n", name);
if (adap->getscl == NULL)
- pr_info("%s: Testing SDA only, SCL is not readable\n", name);
+ pr_info("%s: SCL is write-only, testing not possible\n", name);
- sda = getsda(adap);
- scl = (adap->getscl == NULL) ? 1 : getscl(adap);
+ sda = adap->getsda ? getsda(adap) : 1;
+ scl = adap->getscl ? getscl(adap) : 1;
if (!scl || !sda) {
- printk(KERN_WARNING
- "%s: bus seems to be busy (scl=%d, sda=%d)\n",
- name, scl, sda);
+ pr_warn("%s: bus seems to be busy (scl=%d, sda=%d)\n", name, scl, sda);
goto bailout;
}
sdalo(adap);
- sda = getsda(adap);
- scl = (adap->getscl == NULL) ? 1 : getscl(adap);
- if (sda) {
- printk(KERN_WARNING "%s: SDA stuck high!\n", name);
+ if (adap->getsda && getsda(adap)) {
+ pr_warn("%s: SDA stuck high!\n", name);
goto bailout;
}
- if (!scl) {
- printk(KERN_WARNING
- "%s: SCL unexpected low while pulling SDA low!\n",
- name);
+ if (adap->getscl && !getscl(adap)) {
+ pr_warn("%s: SCL unexpected low while pulling SDA low!\n", name);
goto bailout;
}
sdahi(adap);
- sda = getsda(adap);
- scl = (adap->getscl == NULL) ? 1 : getscl(adap);
- if (!sda) {
- printk(KERN_WARNING "%s: SDA stuck low!\n", name);
+ if (adap->getsda && !getsda(adap)) {
+ pr_warn("%s: SDA stuck low!\n", name);
goto bailout;
}
- if (!scl) {
- printk(KERN_WARNING
- "%s: SCL unexpected low while pulling SDA high!\n",
- name);
+ if (adap->getscl && !getscl(adap)) {
+ pr_warn("%s: SCL unexpected low while pulling SDA high!\n", name);
goto bailout;
}
scllo(adap);
- sda = getsda(adap);
- scl = (adap->getscl == NULL) ? 0 : getscl(adap);
- if (scl) {
- printk(KERN_WARNING "%s: SCL stuck high!\n", name);
+ if (adap->getscl && getscl(adap)) {
+ pr_warn("%s: SCL stuck high!\n", name);
goto bailout;
}
- if (!sda) {
- printk(KERN_WARNING
- "%s: SDA unexpected low while pulling SCL low!\n",
- name);
+ if (adap->getsda && !getsda(adap)) {
+ pr_warn("%s: SDA unexpected low while pulling SCL low!\n", name);
goto bailout;
}
sclhi(adap);
- sda = getsda(adap);
- scl = (adap->getscl == NULL) ? 1 : getscl(adap);
- if (!scl) {
- printk(KERN_WARNING "%s: SCL stuck low!\n", name);
+ if (adap->getscl && !getscl(adap)) {
+ pr_warn("%s: SCL stuck low!\n", name);
goto bailout;
}
- if (!sda) {
- printk(KERN_WARNING
- "%s: SDA unexpected low while pulling SCL high!\n",
- name);
+ if (adap->getsda && !getsda(adap)) {
+ pr_warn("%s: SDA unexpected low while pulling SCL high!\n", name);
goto bailout;
}
@@ -420,6 +405,10 @@ static int readbytes(struct i2c_adapter *i2c_adap, struct i2c_msg *msg)
unsigned char *temp = msg->buf;
int count = msg->len;
const unsigned flags = msg->flags;
+ struct i2c_algo_bit_data *adap = i2c_adap->algo_data;
+
+ if (!adap->getsda)
+ return -EOPNOTSUPP;
while (count > 0) {
inval = i2c_inb(i2c_adap);
@@ -670,11 +659,15 @@ static int __i2c_bit_add_bus(struct i2c_adapter *adap,
if (ret < 0)
return ret;
- /* Complain if SCL can't be read */
- if (bit_adap->getscl == NULL) {
+ if (bit_adap->getsda == NULL)
+ dev_warn(&adap->dev, "Not I2C compliant: can't read SDA\n");
+
+ if (bit_adap->getscl == NULL)
dev_warn(&adap->dev, "Not I2C compliant: can't read SCL\n");
+
+ if (bit_adap->getsda == NULL || bit_adap->getscl == NULL)
dev_warn(&adap->dev, "Bus may be unreliable\n");
- }
+
return 0;
}
--
2.39.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v4 3/3] i2c: gpio: support write-only sda/scl w/o pull-up
2023-01-17 20:34 [PATCH v4 0/3] i2c: gpio: support write-only sda Heiner Kallweit
2023-01-17 20:36 ` [PATCH v4 1/3] dt-bindings: i2c-gpio: Add properties for dealing with write-only SDA/SCL w/o pullup Heiner Kallweit
2023-01-17 20:38 ` [PATCH v4 2/3] i2c: algo: bit: allow getsda to be NULL Heiner Kallweit
@ 2023-01-17 20:39 ` Heiner Kallweit
2 siblings, 0 replies; 6+ messages in thread
From: Heiner Kallweit @ 2023-01-17 20:39 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Wolfram Sang, Peter Rosin
Cc: linux-i2c, devicetree
There are slave devices that understand I2C but have read-only SDA and
SCL. Examples are FD650 7-segment LED controller and its derivatives.
Typical board designs don't even have a pull-up for both pins.
Handle the new attributes for write-only SDA and missing pull-up on
SDA/SCL.
For either pin the open-drain and has-no-pullup properties are
mutually-exclusive, what is documented in the DT property documentation.
We don't add an extra warning here because the open-drain properties
are marked deprecated anyway.
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
v3:
- improve description of attribute sda_is_output_only
v4:
- handle new no-pullup attributes
---
drivers/i2c/busses/i2c-gpio.c | 13 ++++++++++---
include/linux/platform_data/i2c-gpio.h | 9 +++++++++
2 files changed, 19 insertions(+), 3 deletions(-)
diff --git a/drivers/i2c/busses/i2c-gpio.c b/drivers/i2c/busses/i2c-gpio.c
index 0e4385a9b..85b3beb20 100644
--- a/drivers/i2c/busses/i2c-gpio.c
+++ b/drivers/i2c/busses/i2c-gpio.c
@@ -316,6 +316,12 @@ static void of_i2c_gpio_get_props(struct device_node *np,
of_property_read_bool(np, "i2c-gpio,scl-open-drain");
pdata->scl_is_output_only =
of_property_read_bool(np, "i2c-gpio,scl-output-only");
+ pdata->sda_is_output_only =
+ of_property_read_bool(np, "i2c-gpio,sda-output-only");
+ pdata->sda_has_no_pullup =
+ of_property_read_bool(np, "i2c-gpio,sda-has-no-pullup");
+ pdata->scl_has_no_pullup =
+ of_property_read_bool(np, "i2c-gpio,scl-has-no-pullup");
}
static struct gpio_desc *i2c_gpio_get_desc(struct device *dev,
@@ -392,7 +398,7 @@ static int i2c_gpio_probe(struct platform_device *pdev)
* handle them as we handle any other output. Else we enforce open
* drain as this is required for an I2C bus.
*/
- if (pdata->sda_is_open_drain)
+ if (pdata->sda_is_open_drain || pdata->sda_has_no_pullup)
gflags = GPIOD_OUT_HIGH;
else
gflags = GPIOD_OUT_HIGH_OPEN_DRAIN;
@@ -400,7 +406,7 @@ static int i2c_gpio_probe(struct platform_device *pdev)
if (IS_ERR(priv->sda))
return PTR_ERR(priv->sda);
- if (pdata->scl_is_open_drain)
+ if (pdata->scl_is_open_drain || pdata->scl_has_no_pullup)
gflags = GPIOD_OUT_HIGH;
else
gflags = GPIOD_OUT_HIGH_OPEN_DRAIN;
@@ -418,7 +424,8 @@ static int i2c_gpio_probe(struct platform_device *pdev)
if (!pdata->scl_is_output_only)
bit_data->getscl = i2c_gpio_getscl;
- bit_data->getsda = i2c_gpio_getsda;
+ if (!pdata->sda_is_output_only)
+ bit_data->getsda = i2c_gpio_getsda;
if (pdata->udelay)
bit_data->udelay = pdata->udelay;
diff --git a/include/linux/platform_data/i2c-gpio.h b/include/linux/platform_data/i2c-gpio.h
index a907774fd..545639bcc 100644
--- a/include/linux/platform_data/i2c-gpio.h
+++ b/include/linux/platform_data/i2c-gpio.h
@@ -16,16 +16,25 @@
* isn't actively driven high when setting the output value high.
* gpio_get_value() must return the actual pin state even if the
* pin is configured as an output.
+ * @sda_is_output_only: SDA output drivers can't be turned off.
+ * This is for clients that can only read SDA/SCL.
+ * @sda_has_no_pullup: SDA is used in a non-compliant way and has no pull-up.
+ * Therefore disable open-drain.
* @scl_is_open_drain: SCL is set up as open drain. Same requirements
* as for sda_is_open_drain apply.
* @scl_is_output_only: SCL output drivers cannot be turned off.
+ * @scl_has_no_pullup: SCL is used in a non-compliant way and has no pull-up.
+ * Therefore disable open-drain.
*/
struct i2c_gpio_platform_data {
int udelay;
int timeout;
unsigned int sda_is_open_drain:1;
+ unsigned int sda_is_output_only:1;
+ unsigned int sda_has_no_pullup:1;
unsigned int scl_is_open_drain:1;
unsigned int scl_is_output_only:1;
+ unsigned int scl_has_no_pullup:1;
};
#endif /* _LINUX_I2C_GPIO_H */
--
2.39.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v4 1/3] dt-bindings: i2c-gpio: Add properties for dealing with write-only SDA/SCL w/o pullup
2023-01-17 20:36 ` [PATCH v4 1/3] dt-bindings: i2c-gpio: Add properties for dealing with write-only SDA/SCL w/o pullup Heiner Kallweit
@ 2023-01-18 16:32 ` Rob Herring
2023-01-18 17:55 ` Heiner Kallweit
0 siblings, 1 reply; 6+ messages in thread
From: Rob Herring @ 2023-01-18 16:32 UTC (permalink / raw)
To: Heiner Kallweit
Cc: Krzysztof Kozlowski, Wolfram Sang, Peter Rosin, linux-i2c, devicetree
On Tue, Jan 17, 2023 at 09:36:05PM +0100, Heiner Kallweit wrote:
> There are slave devices that understand I2C but have read-only SDA and
> SCL. Examples are FD650 7-segment LED controller and its derivatives.
> Typical board designs don't even have a pull-up for both pins.
> Therefore add properties for not using open-drain. For write-only SCL
> we have a property already, add one for write-only SDA.
>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> ---
> v4:
> - add no-pullup properties
> ---
> .../devicetree/bindings/i2c/i2c-gpio.yaml | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/i2c/i2c-gpio.yaml b/Documentation/devicetree/bindings/i2c/i2c-gpio.yaml
> index e0d76d5eb..67898cc52 100644
> --- a/Documentation/devicetree/bindings/i2c/i2c-gpio.yaml
> +++ b/Documentation/devicetree/bindings/i2c/i2c-gpio.yaml
> @@ -33,6 +33,10 @@ properties:
> open drain.
> maxItems: 1
>
> + i2c-gpio,sda-output-only:
> + description: sda as output only
> + type: boolean
> +
> i2c-gpio,scl-output-only:
> description: scl as output only
> type: boolean
> @@ -63,6 +67,18 @@ properties:
> GPIO line used for SCL into open drain mode, and that something is not
> the GPIO chip. It is essentially an inconsistency flag.
>
> + i2c-gpio,sda-has-no-pullup:
> + type: boolean
> + description: sda is used in a non-compliant way and has no pull-up.
> + Therefore disable open-drain. This property is mutually-exclusive
> + with i2c-gpio,sda-open-drain.
> +
> + i2c-gpio,scl-has-no-pullup:
> + type: boolean
> + description: scl is used in a non-compliant way and has no pull-up.
> + Therefore disable open-drain. This property is mutually-exclusive
> + with i2c-gpio,scl-open-drain.
The mutual-exclusion can be expressed as a schema instead:
allOf:
- not:
required:
- i2c-gpio,scl-has-no-pullup
- i2c-gpio,scl-open-drain
- not:
required:
- i2c-gpio,sda-has-no-pullup
- i2c-gpio,sda-open-drain
Using 'dependencies' with a schema would also work.
Rob
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v4 1/3] dt-bindings: i2c-gpio: Add properties for dealing with write-only SDA/SCL w/o pullup
2023-01-18 16:32 ` Rob Herring
@ 2023-01-18 17:55 ` Heiner Kallweit
0 siblings, 0 replies; 6+ messages in thread
From: Heiner Kallweit @ 2023-01-18 17:55 UTC (permalink / raw)
To: Rob Herring
Cc: Krzysztof Kozlowski, Wolfram Sang, Peter Rosin, linux-i2c, devicetree
On 18.01.2023 17:32, Rob Herring wrote:
> On Tue, Jan 17, 2023 at 09:36:05PM +0100, Heiner Kallweit wrote:
>> There are slave devices that understand I2C but have read-only SDA and
>> SCL. Examples are FD650 7-segment LED controller and its derivatives.
>> Typical board designs don't even have a pull-up for both pins.
>> Therefore add properties for not using open-drain. For write-only SCL
>> we have a property already, add one for write-only SDA.
>>
>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>> ---
>> v4:
>> - add no-pullup properties
>> ---
>> .../devicetree/bindings/i2c/i2c-gpio.yaml | 16 ++++++++++++++++
>> 1 file changed, 16 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/i2c/i2c-gpio.yaml b/Documentation/devicetree/bindings/i2c/i2c-gpio.yaml
>> index e0d76d5eb..67898cc52 100644
>> --- a/Documentation/devicetree/bindings/i2c/i2c-gpio.yaml
>> +++ b/Documentation/devicetree/bindings/i2c/i2c-gpio.yaml
>> @@ -33,6 +33,10 @@ properties:
>> open drain.
>> maxItems: 1
>>
>> + i2c-gpio,sda-output-only:
>> + description: sda as output only
>> + type: boolean
>> +
>> i2c-gpio,scl-output-only:
>> description: scl as output only
>> type: boolean
>> @@ -63,6 +67,18 @@ properties:
>> GPIO line used for SCL into open drain mode, and that something is not
>> the GPIO chip. It is essentially an inconsistency flag.
>>
>> + i2c-gpio,sda-has-no-pullup:
>> + type: boolean
>> + description: sda is used in a non-compliant way and has no pull-up.
>> + Therefore disable open-drain. This property is mutually-exclusive
>> + with i2c-gpio,sda-open-drain.
>> +
>> + i2c-gpio,scl-has-no-pullup:
>> + type: boolean
>> + description: scl is used in a non-compliant way and has no pull-up.
>> + Therefore disable open-drain. This property is mutually-exclusive
>> + with i2c-gpio,scl-open-drain.
>
> The mutual-exclusion can be expressed as a schema instead:
>
> allOf:
> - not:
> required:
> - i2c-gpio,scl-has-no-pullup
> - i2c-gpio,scl-open-drain
> - not:
> required:
> - i2c-gpio,sda-has-no-pullup
> - i2c-gpio,sda-open-drain
>
> Using 'dependencies' with a schema would also work.
>
Great, I wasn't aware of options to express mutually-exclusive properties.
I'll wait a little for other feedback regarding the series and then submit
a new version incl. the extended schema.
> Rob
Heiner
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-01-18 17:55 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-17 20:34 [PATCH v4 0/3] i2c: gpio: support write-only sda Heiner Kallweit
2023-01-17 20:36 ` [PATCH v4 1/3] dt-bindings: i2c-gpio: Add properties for dealing with write-only SDA/SCL w/o pullup Heiner Kallweit
2023-01-18 16:32 ` Rob Herring
2023-01-18 17:55 ` Heiner Kallweit
2023-01-17 20:38 ` [PATCH v4 2/3] i2c: algo: bit: allow getsda to be NULL Heiner Kallweit
2023-01-17 20:39 ` [PATCH v4 3/3] i2c: gpio: support write-only sda/scl w/o pull-up Heiner Kallweit
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.