[v2] usb: typec: fsa4480: Check if the chip is really there

[PATCH v2] usb: typec: fsa4480: Check if the chip is really there

Posted by Konrad Dybcio 1 month, 1 week ago

From: Konrad Dybcio <konrad.dybcio@linaro.org>

Currently, the driver will happily register the switch/mux devices, and
so long as the i2c master doesn't complain, the user would never know
there's something wrong.

Add a device id check (based on [1]) and return -ENODEV if the read
fails or returns nonsense.

Checking the value on a Qualcomm SM6115P-based Lenovo Tab P11 tablet,
the ID mentioned in the datasheet does indeed show up:
 fsa4480 1-0042: Found FSA4480 v1.1 (Vendor ID = 0)

[1] https://www.onsemi.com/pdf/datasheet/fsa4480-d.pdf

Fixes: 1dc246320c6b ("usb: typec: mux: Add On Semi fsa4480 driver")
Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
Signed-off-by: Konrad Dybcio <konradybcio@kernel.org>
---
Changes in v2:
- Prepend the new defines with FSA4480_ to make them more obvious
- Link to v1: https://lore.kernel.org/r/20240212-topic-fs4480_check-v1-1-d9969e4d6f9a@linaro.org
---
 drivers/usb/typec/mux/fsa4480.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/usb/typec/mux/fsa4480.c b/drivers/usb/typec/mux/fsa4480.c
index cb7cdf90cb0a..cd235339834b 100644
--- a/drivers/usb/typec/mux/fsa4480.c
+++ b/drivers/usb/typec/mux/fsa4480.c
@@ -13,6 +13,10 @@
 #include <linux/usb/typec_dp.h>
 #include <linux/usb/typec_mux.h>
 
+#define FSA4480_DEVICE_ID	0x00
+ #define FSA4480_DEVICE_ID_VENDOR_ID	GENMASK(7, 6)
+ #define FSA4480_DEVICE_ID_VERSION_ID	GENMASK(5, 3)
+ #define FSA4480_DEVICE_ID_REV_ID	GENMASK(2, 0)
 #define FSA4480_SWITCH_ENABLE	0x04
 #define FSA4480_SWITCH_SELECT	0x05
 #define FSA4480_SWITCH_STATUS1	0x07
@@ -251,6 +255,7 @@ static int fsa4480_probe(struct i2c_client *client)
 	struct typec_switch_desc sw_desc = { };
 	struct typec_mux_desc mux_desc = { };
 	struct fsa4480 *fsa;
+	int val = 0;
 	int ret;
 
 	fsa = devm_kzalloc(dev, sizeof(*fsa), GFP_KERNEL);
@@ -268,6 +273,15 @@ static int fsa4480_probe(struct i2c_client *client)
 	if (IS_ERR(fsa->regmap))
 		return dev_err_probe(dev, PTR_ERR(fsa->regmap), "failed to initialize regmap\n");
 
+	ret = regmap_read(fsa->regmap, FSA4480_DEVICE_ID, &val);
+	if (ret || !val)
+		return dev_err_probe(dev, -ENODEV, "FSA4480 not found\n");
+
+	dev_dbg(dev, "Found FSA4480 v%lu.%lu (Vendor ID = %lu)\n",
+		FIELD_GET(FSA4480_DEVICE_ID_VERSION_ID, val),
+		FIELD_GET(FSA4480_DEVICE_ID_REV_ID, val),
+		FIELD_GET(FSA4480_DEVICE_ID_VENDOR_ID, val));
+
 	/* Safe mode */
 	fsa->cur_enable = FSA4480_ENABLE_DEVICE | FSA4480_ENABLE_USB;
 	fsa->mode = TYPEC_STATE_SAFE;

---
base-commit: 2347b4c79f5e6cd3f4996e80c2d3c15f53006bf5
change-id: 20240212-topic-fs4480_check-caa0160fb101

Best regards,
-- 
Konrad Dybcio <konradybcio@kernel.org>

Re: [PATCH v2] usb: typec: fsa4480: Check if the chip is really there

Posted by Dmitry Baryshkov 1 month, 1 week ago

On Fri, Jul 26, 2024 at 01:43:30PM GMT, Konrad Dybcio wrote:
> From: Konrad Dybcio <konrad.dybcio@linaro.org>
> 
> Currently, the driver will happily register the switch/mux devices, and
> so long as the i2c master doesn't complain, the user would never know
> there's something wrong.
> 
> Add a device id check (based on [1]) and return -ENODEV if the read
> fails or returns nonsense.
> 
> Checking the value on a Qualcomm SM6115P-based Lenovo Tab P11 tablet,
> the ID mentioned in the datasheet does indeed show up:
>  fsa4480 1-0042: Found FSA4480 v1.1 (Vendor ID = 0)

So wonderful to have 0 Vendor ID (initially assumed that you are showing
it as an example of an error). But yes, the datasheet has 0 there.

> 
> [1] https://www.onsemi.com/pdf/datasheet/fsa4480-d.pdf
> 
> Fixes: 1dc246320c6b ("usb: typec: mux: Add On Semi fsa4480 driver")
> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
> Signed-off-by: Konrad Dybcio <konradybcio@kernel.org>
> ---
> Changes in v2:
> - Prepend the new defines with FSA4480_ to make them more obvious
> - Link to v1: https://lore.kernel.org/r/20240212-topic-fs4480_check-v1-1-d9969e4d6f9a@linaro.org
> ---
>  drivers/usb/typec/mux/fsa4480.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)

With the S-o-B tags fixed:

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

-- 
With best wishes
Dmitry

Re: [PATCH v2] usb: typec: fsa4480: Check if the chip is really there

Posted by Greg Kroah-Hartman 1 month, 1 week ago

On Fri, Jul 26, 2024 at 01:43:30PM +0200, Konrad Dybcio wrote:
> From: Konrad Dybcio <konrad.dybcio@linaro.org>
> 
> Currently, the driver will happily register the switch/mux devices, and
> so long as the i2c master doesn't complain, the user would never know
> there's something wrong.
> 
> Add a device id check (based on [1]) and return -ENODEV if the read
> fails or returns nonsense.
> 
> Checking the value on a Qualcomm SM6115P-based Lenovo Tab P11 tablet,
> the ID mentioned in the datasheet does indeed show up:
>  fsa4480 1-0042: Found FSA4480 v1.1 (Vendor ID = 0)
> 
> [1] https://www.onsemi.com/pdf/datasheet/fsa4480-d.pdf
> 
> Fixes: 1dc246320c6b ("usb: typec: mux: Add On Semi fsa4480 driver")
> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
> Signed-off-by: Konrad Dybcio <konradybcio@kernel.org>

You can't sign off on a patch twice, that makes no sense, sorry.

greg k-h

Re: [PATCH v2] usb: typec: fsa4480: Check if the chip is really there

Posted by Konrad Dybcio 1 month, 1 week ago


On 26.07.2024 3:12 PM, Greg Kroah-Hartman wrote:
> On Fri, Jul 26, 2024 at 01:43:30PM +0200, Konrad Dybcio wrote:
>> From: Konrad Dybcio <konrad.dybcio@linaro.org>
>>
>> Currently, the driver will happily register the switch/mux devices, and
>> so long as the i2c master doesn't complain, the user would never know
>> there's something wrong.
>>
>> Add a device id check (based on [1]) and return -ENODEV if the read
>> fails or returns nonsense.
>>
>> Checking the value on a Qualcomm SM6115P-based Lenovo Tab P11 tablet,
>> the ID mentioned in the datasheet does indeed show up:
>>  fsa4480 1-0042: Found FSA4480 v1.1 (Vendor ID = 0)
>>
>> [1] https://www.onsemi.com/pdf/datasheet/fsa4480-d.pdf
>>
>> Fixes: 1dc246320c6b ("usb: typec: mux: Add On Semi fsa4480 driver")
>> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
>> Signed-off-by: Konrad Dybcio <konradybcio@kernel.org>
> You can't sign off on a patch twice, that makes no sense, sorry.

I'm losing access to the @linaro.org email and want to preserve the
authorship there (as this patch was developed during work hours).

Then, the author's email doesn't match the sender's email, so I'm
expected to sign off with the sender's one.

Should I assume that the maintainer trusts me to be the same person?

Konrad

Re: [PATCH v2] usb: typec: fsa4480: Check if the chip is really there

Posted by Greg Kroah-Hartman 1 month, 1 week ago

On Fri, Jul 26, 2024 at 03:52:22PM +0200, Konrad Dybcio wrote:
> 
> 
> On 26.07.2024 3:12 PM, Greg Kroah-Hartman wrote:
> > On Fri, Jul 26, 2024 at 01:43:30PM +0200, Konrad Dybcio wrote:
> >> From: Konrad Dybcio <konrad.dybcio@linaro.org>
> >>
> >> Currently, the driver will happily register the switch/mux devices, and
> >> so long as the i2c master doesn't complain, the user would never know
> >> there's something wrong.
> >>
> >> Add a device id check (based on [1]) and return -ENODEV if the read
> >> fails or returns nonsense.
> >>
> >> Checking the value on a Qualcomm SM6115P-based Lenovo Tab P11 tablet,
> >> the ID mentioned in the datasheet does indeed show up:
> >>  fsa4480 1-0042: Found FSA4480 v1.1 (Vendor ID = 0)
> >>
> >> [1] https://www.onsemi.com/pdf/datasheet/fsa4480-d.pdf
> >>
> >> Fixes: 1dc246320c6b ("usb: typec: mux: Add On Semi fsa4480 driver")
> >> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
> >> Signed-off-by: Konrad Dybcio <konradybcio@kernel.org>
> > You can't sign off on a patch twice, that makes no sense, sorry.
> 
> I'm losing access to the @linaro.org email and want to preserve the
> authorship there (as this patch was developed during work hours).
> 
> Then, the author's email doesn't match the sender's email, so I'm
> expected to sign off with the sender's one.
> 
> Should I assume that the maintainer trusts me to be the same person?

Yes I do :)

Re: [PATCH v2] usb: typec: fsa4480: Check if the chip is really there

Posted by Bjorn Andersson 1 month, 1 week ago

On Fri, Jul 26, 2024 at 03:52:22PM GMT, Konrad Dybcio wrote:
> 
> 
> On 26.07.2024 3:12 PM, Greg Kroah-Hartman wrote:
> > On Fri, Jul 26, 2024 at 01:43:30PM +0200, Konrad Dybcio wrote:
> >> From: Konrad Dybcio <konrad.dybcio@linaro.org>
> >>
> >> Currently, the driver will happily register the switch/mux devices, and
> >> so long as the i2c master doesn't complain, the user would never know
> >> there's something wrong.
> >>
> >> Add a device id check (based on [1]) and return -ENODEV if the read
> >> fails or returns nonsense.
> >>
> >> Checking the value on a Qualcomm SM6115P-based Lenovo Tab P11 tablet,
> >> the ID mentioned in the datasheet does indeed show up:
> >>  fsa4480 1-0042: Found FSA4480 v1.1 (Vendor ID = 0)
> >>
> >> [1] https://www.onsemi.com/pdf/datasheet/fsa4480-d.pdf
> >>
> >> Fixes: 1dc246320c6b ("usb: typec: mux: Add On Semi fsa4480 driver")
> >> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
> >> Signed-off-by: Konrad Dybcio <konradybcio@kernel.org>
> > You can't sign off on a patch twice, that makes no sense, sorry.
> 
> I'm losing access to the @linaro.org email and want to preserve the
> authorship there (as this patch was developed during work hours).
> 
> Then, the author's email doesn't match the sender's email, so I'm
> expected to sign off with the sender's one.
> 

The author is Linaro and as such the first s-o-b is correct/required.

> Should I assume that the maintainer trusts me to be the same person?
> 

I think in many cases you can assume that, but I find it reasonable that
you certify the origin of the patch anew, even though you happen to be
the same physical person.

Regards,
Bjorn