[PATCH v3 07/24] iio: accel: kxcjk-1013: Revert "Add support for KX022-1020"

Andy Shevchenko posted 24 patches 1 month ago
[PATCH v3 07/24] iio: accel: kxcjk-1013: Revert "Add support for KX022-1020"
Posted by Andy Shevchenko 1 month ago
The mentioned change effectively broke the ODR startup timeouts
settungs for KX023-1025 case. Let's revert it for now and see
how we can handle it with the better approach after switching
the driver to use data structure instead of enum.

This reverts commit d5cbe1502043124ff8af8136b80f93758c4a61e0.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/iio/accel/kxcjk-1013.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/iio/accel/kxcjk-1013.c b/drivers/iio/accel/kxcjk-1013.c
index 2eec95d8defb..208e701e1aed 100644
--- a/drivers/iio/accel/kxcjk-1013.c
+++ b/drivers/iio/accel/kxcjk-1013.c
@@ -174,7 +174,6 @@ enum kx_chipset {
 	KXCJ91008,
 	KXTJ21009,
 	KXTF9,
-	KX0221020,
 	KX0231025,
 	KX_MAX_CHIPS /* this must be last */
 };
@@ -582,8 +581,8 @@ static int kxcjk1013_chip_init(struct kxcjk1013_data *data)
 		return ret;
 	}
 
-	/* On KX023 and KX022, route all used interrupts to INT1 for now */
-	if ((data->chipset == KX0231025 || data->chipset == KX0221020) && data->client->irq > 0) {
+	/* On KX023, route all used interrupts to INT1 for now */
+	if (data->chipset == KX0231025 && data->client->irq > 0) {
 		ret = i2c_smbus_write_byte_data(data->client, KX023_REG_INC4,
 						KX023_REG_INC4_DRDY1 |
 						KX023_REG_INC4_WUFI1);
@@ -1509,7 +1508,6 @@ static int kxcjk1013_probe(struct i2c_client *client)
 	case KXTF9:
 		data->regs = &kxtf9_regs;
 		break;
-	case KX0221020:
 	case KX0231025:
 		data->regs = &kx0231025_regs;
 		break;
@@ -1715,7 +1713,6 @@ static const struct i2c_device_id kxcjk1013_id[] = {
 	{"kxcj91008", KXCJ91008},
 	{"kxtj21009", KXTJ21009},
 	{"kxtf9",     KXTF9},
-	{"kx022-1020", KX0221020},
 	{"kx023-1025", KX0231025},
 	{}
 };
@@ -1727,7 +1724,6 @@ static const struct of_device_id kxcjk1013_of_match[] = {
 	{ .compatible = "kionix,kxcj91008", },
 	{ .compatible = "kionix,kxtj21009", },
 	{ .compatible = "kionix,kxtf9", },
-	{ .compatible = "kionix,kx022-1020", },
 	{ .compatible = "kionix,kx023-1025", },
 	{ }
 };
-- 
2.43.0.rc1.1336.g36b5255a03ac
Re: [PATCH v3 07/24] iio: accel: kxcjk-1013: Revert "Add support for KX022-1020"
Posted by Jonathan Cameron 1 month ago
On Thu, 24 Oct 2024 22:04:56 +0300
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> The mentioned change effectively broke the ODR startup timeouts
> settungs for KX023-1025 case. Let's revert it for now and see
> how we can handle it with the better approach after switching
> the driver to use data structure instead of enum.
> 
> This reverts commit d5cbe1502043124ff8af8136b80f93758c4a61e0.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
I'll take this the slow way as I don't think there is time to chase the revert
through the various trees and still get the dependent patches in.
Hopefully we will fairly quickly get the missing table data and can
bring this back again.

For now, applied to the togreg branch of iio.git.  
I have tagged it as a fix though. and +CC Rayyan
(I'm guessing maybe that will bounce as you rarely miss people you should
CC!)

Jonathan

> ---
>  drivers/iio/accel/kxcjk-1013.c | 8 ++------
>  1 file changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/iio/accel/kxcjk-1013.c b/drivers/iio/accel/kxcjk-1013.c
> index 2eec95d8defb..208e701e1aed 100644
> --- a/drivers/iio/accel/kxcjk-1013.c
> +++ b/drivers/iio/accel/kxcjk-1013.c
> @@ -174,7 +174,6 @@ enum kx_chipset {
>  	KXCJ91008,
>  	KXTJ21009,
>  	KXTF9,
> -	KX0221020,
>  	KX0231025,
>  	KX_MAX_CHIPS /* this must be last */
>  };
> @@ -582,8 +581,8 @@ static int kxcjk1013_chip_init(struct kxcjk1013_data *data)
>  		return ret;
>  	}
>  
> -	/* On KX023 and KX022, route all used interrupts to INT1 for now */
> -	if ((data->chipset == KX0231025 || data->chipset == KX0221020) && data->client->irq > 0) {
> +	/* On KX023, route all used interrupts to INT1 for now */
> +	if (data->chipset == KX0231025 && data->client->irq > 0) {
>  		ret = i2c_smbus_write_byte_data(data->client, KX023_REG_INC4,
>  						KX023_REG_INC4_DRDY1 |
>  						KX023_REG_INC4_WUFI1);
> @@ -1509,7 +1508,6 @@ static int kxcjk1013_probe(struct i2c_client *client)
>  	case KXTF9:
>  		data->regs = &kxtf9_regs;
>  		break;
> -	case KX0221020:
>  	case KX0231025:
>  		data->regs = &kx0231025_regs;
>  		break;
> @@ -1715,7 +1713,6 @@ static const struct i2c_device_id kxcjk1013_id[] = {
>  	{"kxcj91008", KXCJ91008},
>  	{"kxtj21009", KXTJ21009},
>  	{"kxtf9",     KXTF9},
> -	{"kx022-1020", KX0221020},
>  	{"kx023-1025", KX0231025},
>  	{}
>  };
> @@ -1727,7 +1724,6 @@ static const struct of_device_id kxcjk1013_of_match[] = {
>  	{ .compatible = "kionix,kxcj91008", },
>  	{ .compatible = "kionix,kxtj21009", },
>  	{ .compatible = "kionix,kxtf9", },
> -	{ .compatible = "kionix,kx022-1020", },
>  	{ .compatible = "kionix,kx023-1025", },
>  	{ }
>  };
Re: [PATCH v3 07/24] iio: accel: kxcjk-1013: Revert "Add support for KX022-1020"
Posted by Rayyan Ansari 1 month ago
On 26/10/2024 12:16, Jonathan Cameron wrote:
> On Thu, 24 Oct 2024 22:04:56 +0300
> Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> 
>> The mentioned change effectively broke the ODR startup timeouts
>> settungs for KX023-1025 case. Let's revert it for now and see
>> how we can handle it with the better approach after switching
>> the driver to use data structure instead of enum.
>>
>> This reverts commit d5cbe1502043124ff8af8136b80f93758c4a61e0.
>>
>> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> I'll take this the slow way as I don't think there is time to chase the revert
> through the various trees and still get the dependent patches in.
> Hopefully we will fairly quickly get the missing table data and can
> bring this back again.
> 
> For now, applied to the togreg branch of iio.git.
> I have tagged it as a fix though. and +CC Rayyan
> (I'm guessing maybe that will bounce as you rarely miss people you should
> CC!)
Hi,
Sorry for not replying earlier, I've just caught up with the discussion.

I don't fully understand why this is breaking KX023-1025, but you know 
more than I do here.
Does this not mean that the use of KX022-1020 in the 3 devices (Lumia 
640, 640 XL, 735) using this from qcom-msm8226-microsoft-common.dtsi 
will now be broken?

Thanks,
Rayyan
Re: [PATCH v3 07/24] iio: accel: kxcjk-1013: Revert "Add support for KX022-1020"
Posted by Andy Shevchenko 4 weeks ago
On Sat, Oct 26, 2024 at 03:58:52PM +0100, Rayyan Ansari wrote:
> On 26/10/2024 12:16, Jonathan Cameron wrote:
> > On Thu, 24 Oct 2024 22:04:56 +0300
> > Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> > 
> > > The mentioned change effectively broke the ODR startup timeouts
> > > settungs for KX023-1025 case. Let's revert it for now and see
> > > how we can handle it with the better approach after switching
> > > the driver to use data structure instead of enum.
> > > 
> > > This reverts commit d5cbe1502043124ff8af8136b80f93758c4a61e0.
> > > 
> > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > I'll take this the slow way as I don't think there is time to chase the revert
> > through the various trees and still get the dependent patches in.
> > Hopefully we will fairly quickly get the missing table data and can
> > bring this back again.
> > 
> > For now, applied to the togreg branch of iio.git.
> > I have tagged it as a fix though. and +CC Rayyan
> > (I'm guessing maybe that will bounce as you rarely miss people you should
> > CC!)
> Hi,
> Sorry for not replying earlier, I've just caught up with the discussion.
> 
> I don't fully understand why this is breaking KX023-1025, but you know more
> than I do here.
> Does this not mean that the use of KX022-1020 in the 3 devices (Lumia 640,
> 640 XL, 735) using this from qcom-msm8226-microsoft-common.dtsi will now be
> broken?

Jonathan already answered to the question, so, please, rework the patch and
submit it again.

-- 
With Best Regards,
Andy Shevchenko
Re: [PATCH v3 07/24] iio: accel: kxcjk-1013: Revert "Add support for KX022-1020"
Posted by Jonathan Cameron 1 month ago
On Sat, 26 Oct 2024 15:58:52 +0100
Rayyan Ansari <rayyan@ansari.sh> wrote:

> On 26/10/2024 12:16, Jonathan Cameron wrote:
> > On Thu, 24 Oct 2024 22:04:56 +0300
> > Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> >   
> >> The mentioned change effectively broke the ODR startup timeouts
> >> settungs for KX023-1025 case. Let's revert it for now and see
> >> how we can handle it with the better approach after switching
> >> the driver to use data structure instead of enum.
> >>
> >> This reverts commit d5cbe1502043124ff8af8136b80f93758c4a61e0.
> >>
> >> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>  
> > I'll take this the slow way as I don't think there is time to chase the revert
> > through the various trees and still get the dependent patches in.
> > Hopefully we will fairly quickly get the missing table data and can
> > bring this back again.
> > 
> > For now, applied to the togreg branch of iio.git.
> > I have tagged it as a fix though. and +CC Rayyan
> > (I'm guessing maybe that will bounce as you rarely miss people you should
> > CC!)  
> Hi,
> Sorry for not replying earlier, I've just caught up with the discussion.
> 
> I don't fully understand why this is breaking KX023-1025, but you know 
> more than I do here.
> Does this not mean that the use of KX022-1020 in the 3 devices (Lumia 
> 640, 640 XL, 735) using this from qcom-msm8226-microsoft-common.dtsi 
> will now be broken?
> 
Yes.  The issues in the currently driver is here
https://elixir.bootlin.com/linux/v6.12-rc4/source/drivers/iio/accel/kxcjk-1013.c#L321

This array is indexed using the enum
https://elixir.bootlin.com/linux/v6.12-rc4/source/drivers/iio/accel/kxcjk-1013.c#L176
and the new entry for the KX022-1020 mean we are one short of those startup
time definitions.

Without that the values retrieved for the KX022-1025 are all 0.

It should be a relatively easy fix if we have those times.
One side effect of this series of Andy's is that it makes it much harder to have
similar bugs in future. 

Jonathan

> Thanks,
> Rayyan