| Message ID | 20241024191200.229894-8-andriy.shevchenko@linux.intel.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | iio: Clean up acpi_match_device() use cases | expand |
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", }, > { } > };
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
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
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.
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", }, { } };
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(-)