mbox series

[v2,0/3] QCOM QUP I2C - Add noise rejection, convert to YAML

Message ID 20210114174909.399284-1-angelogioacchino.delregno@somainline.org
Headers show
Series QCOM QUP I2C - Add noise rejection, convert to YAML | expand

Message

AngeloGioacchino Del Regno Jan. 14, 2021, 5:49 p.m. UTC
This patch series converts the i2c-qup bindings to YAML and then
adds support for noise rejection, which is needed for some noisy
hardware, like the touchscreen on the F(x)Tec Pro1.
After adding noise rejection, the touchscreen stopped showing
ghost touch issues and lockups.

Tested on F(x)Tec Pro1 (MSM8998).

Changes in v2:
 - Fixed ARM (32) build error (added bitfield.h inclusion)

AngeloGioacchino Del Regno (3):
  dt-bindings: i2c: qcom,i2c-qup: Convert txt to YAML schema
  i2c: qup: Introduce SCL/SDA noise rejection
  dt-bindings: i2c: qcom,i2c-qup: Document noise rejection properties

 .../devicetree/bindings/i2c/qcom,i2c-qup.txt  |  40 -------
 .../devicetree/bindings/i2c/qcom,i2c-qup.yaml | 101 ++++++++++++++++++
 drivers/i2c/busses/i2c-qup.c                  |  17 +++
 3 files changed, 118 insertions(+), 40 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/i2c/qcom,i2c-qup.txt
 create mode 100644 Documentation/devicetree/bindings/i2c/qcom,i2c-qup.yaml

Comments

Bjorn Andersson Jan. 14, 2021, 5:57 p.m. UTC | #1
On Thu 14 Jan 11:49 CST 2021, AngeloGioacchino Del Regno wrote:

> Some I2C devices may be glitchy due to electrical noise coming
> from the device itself or because of possible board design issues.
> To overcome this issue, the QUP's I2C in Qualcomm SoCs supports
> a noise rejection setting for both SCL and SDA lines.
> 
> Introduce a setting for noise rejection through device properties,
> "qcom,noise-reject-sda" and "qcom,noise-reject-scl", which will
> be used to set the level of noise rejection sensitivity.
> If the properties are not specified, noise rejection will not be
> enabled.
> 

This looks reasonable, just some small nits below.

> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@somainline.org>
> ---
>  drivers/i2c/busses/i2c-qup.c | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/drivers/i2c/busses/i2c-qup.c b/drivers/i2c/busses/i2c-qup.c
> index 5a47915869ae..af51234a60ba 100644
> --- a/drivers/i2c/busses/i2c-qup.c
> +++ b/drivers/i2c/busses/i2c-qup.c
> @@ -8,6 +8,7 @@
>  #include <linux/acpi.h>
>  #include <linux/atomic.h>
>  #include <linux/clk.h>
> +#include <linux/bitfield.h>

If you move this one step up you'll maintain the sort order.

>  #include <linux/delay.h>
>  #include <linux/dmaengine.h>
>  #include <linux/dmapool.h>
> @@ -39,6 +40,8 @@
>  #define QUP_MX_READ_CNT		0x208
>  #define QUP_IN_FIFO_BASE	0x218
>  #define QUP_I2C_CLK_CTL		0x400
> +#define  QUP_I2C_CLK_CTL_SDA_NR	GENMASK(27, 26)
> +#define  QUP_I2C_CLK_CTL_SCL_NR	GENMASK(25, 24)
>  #define QUP_I2C_STATUS		0x404
>  #define QUP_I2C_MASTER_GEN	0x408
>  
> @@ -1663,6 +1666,7 @@ static int qup_i2c_probe(struct platform_device *pdev)
>  	int ret, fs_div, hs_div;
>  	u32 src_clk_freq = DEFAULT_SRC_CLK;
>  	u32 clk_freq = DEFAULT_CLK_FREQ;
> +	u32 noise_reject_scl = 0, noise_reject_sda = 0;

You shouldn't need to initialize these, device_property_read_u32() won't
return 0 without updating them.

>  	int blocks;
>  	bool is_qup_v1;
>  
> @@ -1860,6 +1864,19 @@ static int qup_i2c_probe(struct platform_device *pdev)
>  		qup->clk_ctl = ((fs_div / 2) << 16) | (hs_div << 8) | (fs_div & 0xff);
>  	}
>  
> +	/* SCL/SDA Noise rejection (optional) */
> +	ret = device_property_read_u32(qup->dev, "qcom,noise-reject-scl",
> +				      &noise_reject_scl);
> +	if (ret == 0)
> +		qup->clk_ctl |= FIELD_PREP(QUP_I2C_CLK_CTL_SCL_NR,
> +					   noise_reject_scl);

I would prefer if you didn't break this line.

> +
> +	ret = device_property_read_u32(qup->dev, "qcom,noise-reject-sda",
> +				      &noise_reject_sda);
> +	if (ret == 0)
> +		qup->clk_ctl |= FIELD_PREP(QUP_I2C_CLK_CTL_SDA_NR,
> +					   noise_reject_sda);

Ditto.

Regards,
Bjorn

> +
>  	/*
>  	 * Time it takes for a byte to be clocked out on the bus.
>  	 * Each byte takes 9 clock cycles (8 bits + 1 ack).
> -- 
> 2.29.2
>
AngeloGioacchino Del Regno Jan. 14, 2021, 6:05 p.m. UTC | #2
Il 14/01/21 18:58, Bjorn Andersson ha scritto:
> On Thu 14 Jan 11:49 CST 2021, AngeloGioacchino Del Regno wrote:
> 
>> Document the new noise rejection properties "qcom,noise-reject-sda"
>> and "qcom,noise-reject-scl".
>>
> 
> I presume these are unit-less levels?
> 
> 
Yes, there is no unit.

Sorry, I've sent v3 before seeing your R-b on this.

> Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> 
> Regards,
> Bjorn
> 
>> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@somainline.org>
>> ---
>>   .../devicetree/bindings/i2c/qcom,i2c-qup.yaml      | 14 ++++++++++++++
>>   1 file changed, 14 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/i2c/qcom,i2c-qup.yaml b/Documentation/devicetree/bindings/i2c/qcom,i2c-qup.yaml
>> index c5c7db3ac2a6..3f14dd65c6b9 100644
>> --- a/Documentation/devicetree/bindings/i2c/qcom,i2c-qup.yaml
>> +++ b/Documentation/devicetree/bindings/i2c/qcom,i2c-qup.yaml
>> @@ -58,6 +58,20 @@ properties:
>>     '#size-cells':
>>       const: 0
>>   
>> +  qcom,noise-reject-sda:
>> +    $ref: /schemas/types.yaml#/definitions/uint32
>> +    description: Noise rejection level for the SDA line.
>> +    minimum: 0
>> +    maximum: 3
>> +    default: 0
>> +
>> +  qcom,noise-reject-scl:
>> +    $ref: /schemas/types.yaml#/definitions/uint32
>> +    description: Noise rejection level for the SCL line.
>> +    minimum: 0
>> +    maximum: 3
>> +    default: 0
>> +
>>   required:
>>     - compatible
>>     - clocks
>> -- 
>> 2.29.2
>>