* Re: [PATCH 1/2] input: generalize the Imagis touchscreen driver [not found] <7b9864bf-2aa6-4510-ad98-276fbfaadc30@gmail.com> @ 2023-09-27 15:36 ` Markuss Broks 2023-09-28 18:07 ` Karel Balej 0 siblings, 1 reply; 8+ messages in thread From: Markuss Broks @ 2023-09-27 15:36 UTC (permalink / raw) To: Karel Balej Cc: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-input, devicetree, linux-kernel, Duje Mihanović, ~postmarketos/upstreaming, Jeff LaBundy, linmengbo0689 Hi Karel, On 9/26/23 20:35, Karel Balej wrote: > This driver should work with other Imagis ICs of the IST30**C series. > Make that more apparent. To be fair, this driver should work with all Imagis3 chips, which could be a better name for it. However, I agree with Jeff here: the driver doesn't necessarily need renaming. > > Co-developed-by: Duje Mihanović<duje.mihanovic@skole.hr> > Signed-off-by: Duje Mihanović<duje.mihanovic@skole.hr> > Signed-off-by: Karel Balej<balejk@matfyz.cz> > --- > ...gis,ist3038c.yaml => imagis,ist30xxc.yaml} | 2 +- > MAINTAINERS | 2 +- > drivers/input/touchscreen/Kconfig | 4 +- > drivers/input/touchscreen/imagis.c | 86 +++++++++++-------- > 4 files changed, 52 insertions(+), 42 deletions(-) > rename Documentation/devicetree/bindings/input/touchscreen/{imagis,ist3038c.yaml => imagis,ist30xxc.yaml} (99%) > > diff --git a/Documentation/devicetree/bindings/input/touchscreen/imagis,ist3038c.yaml b/Documentation/devicetree/bindings/input/touchscreen/imagis,ist30xxc.yaml > similarity index 99% > rename from Documentation/devicetree/bindings/input/touchscreen/imagis,ist3038c.yaml > rename to Documentation/devicetree/bindings/input/touchscreen/imagis,ist30xxc.yaml > index 0d6b033fd5fb..09bf3a6acc5e 100644 > --- a/Documentation/devicetree/bindings/input/touchscreen/imagis,ist3038c.yaml > +++ b/Documentation/devicetree/bindings/input/touchscreen/imagis,ist30xxc.yaml > @@ -1,7 +1,7 @@ > # SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > %YAML 1.2 > --- > -$id:http://devicetree.org/schemas/input/touchscreen/imagis,ist3038c.yaml# > +$id:http://devicetree.org/schemas/input/touchscreen/imagis,ist30xxc.yaml# > $schema:http://devicetree.org/meta-schemas/core.yaml# > > title: Imagis IST30XXC family touchscreen controller > diff --git a/MAINTAINERS b/MAINTAINERS > index b19995690904..b23e76418d94 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -10209,7 +10209,7 @@ F: drivers/usb/atm/ueagle-atm.c > IMAGIS TOUCHSCREEN DRIVER > M: Markuss Broks<markuss.broks@gmail.com> > S: Maintained > -F: Documentation/devicetree/bindings/input/touchscreen/imagis,ist3038c.yaml > +F: Documentation/devicetree/bindings/input/touchscreen/imagis,ist30xxc.yaml > F: drivers/input/touchscreen/imagis.c > > IMGTEC ASCII LCD DRIVER > diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig > index e3e2324547b9..45503aa2653e 100644 > --- a/drivers/input/touchscreen/Kconfig > +++ b/drivers/input/touchscreen/Kconfig > @@ -665,10 +665,10 @@ config TOUCHSCREEN_NOVATEK_NVT_TS > module will be called novatek-nvt-ts. > > config TOUCHSCREEN_IMAGIS > - tristate "Imagis touchscreen support" > + tristate "Imagis IST30XXC touchscreen support" > depends on I2C > help > - Say Y here if you have an Imagis IST30xxC touchscreen. > + Say Y here if you have an Imagis IST30XXC touchscreen. > If unsure, say N. > > To compile this driver as a module, choose M here: the > diff --git a/drivers/input/touchscreen/imagis.c b/drivers/input/touchscreen/imagis.c > index 07111ca24455..4456f1b4d527 100644 > --- a/drivers/input/touchscreen/imagis.c > +++ b/drivers/input/touchscreen/imagis.c > @@ -11,25 +11,26 @@ > #include <linux/property.h> > #include <linux/regulator/consumer.h> > > -#define IST3038C_HIB_ACCESS (0x800B << 16) > -#define IST3038C_DIRECT_ACCESS BIT(31) > -#define IST3038C_REG_CHIPID 0x40001000 > -#define IST3038C_REG_HIB_BASE 0x30000100 > -#define IST3038C_REG_TOUCH_STATUS (IST3038C_REG_HIB_BASE | IST3038C_HIB_ACCESS) > -#define IST3038C_REG_TOUCH_COORD (IST3038C_REG_HIB_BASE | IST3038C_HIB_ACCESS | 0x8) > -#define IST3038C_REG_INTR_MESSAGE (IST3038C_REG_HIB_BASE | IST3038C_HIB_ACCESS | 0x4) > +#define IST30XXC_HIB_ACCESS (0x800B << 16) > +#define IST30XXC_DIRECT_ACCESS BIT(31) > +#define IST30XXC_REG_CHIPID 0x40001000 > +#define IST30XXC_REG_HIB_BASE 0x30000100 > +#define IST30XXC_REG_TOUCH_STATUS (IST30XXC_REG_HIB_BASE | IST30XXC_HIB_ACCESS) > +#define IST30XXC_REG_TOUCH_COORD (IST30XXC_REG_HIB_BASE | IST30XXC_HIB_ACCESS | 0x8) > +#define IST30XXC_REG_INTR_MESSAGE (IST30XXC_REG_HIB_BASE | IST30XXC_HIB_ACCESS | 0x4) > +#define IST30XXC_CHIP_ON_DELAY_MS 60 > +#define IST30XXC_I2C_RETRY_COUNT 3 > +#define IST30XXC_MAX_FINGER_NUM 10 > +#define IST30XXC_X_MASK GENMASK(23, 12) > +#define IST30XXC_X_SHIFT 12 > +#define IST30XXC_Y_MASK GENMASK(11, 0) > +#define IST30XXC_AREA_MASK GENMASK(27, 24) > +#define IST30XXC_AREA_SHIFT 24 > +#define IST30XXC_FINGER_COUNT_MASK GENMASK(15, 12) > +#define IST30XXC_FINGER_COUNT_SHIFT 12 > +#define IST30XXC_FINGER_STATUS_MASK GENMASK(9, 0) > + > #define IST3038C_WHOAMI 0x38c > -#define IST3038C_CHIP_ON_DELAY_MS 60 > -#define IST3038C_I2C_RETRY_COUNT 3 > -#define IST3038C_MAX_FINGER_NUM 10 > -#define IST3038C_X_MASK GENMASK(23, 12) > -#define IST3038C_X_SHIFT 12 > -#define IST3038C_Y_MASK GENMASK(11, 0) > -#define IST3038C_AREA_MASK GENMASK(27, 24) > -#define IST3038C_AREA_SHIFT 24 > -#define IST3038C_FINGER_COUNT_MASK GENMASK(15, 12) > -#define IST3038C_FINGER_COUNT_SHIFT 12 > -#define IST3038C_FINGER_STATUS_MASK GENMASK(9, 0) > > struct imagis_ts { > struct i2c_client *client; > @@ -57,7 +58,7 @@ static int imagis_i2c_read_reg(struct imagis_ts *ts, > }, > }; > int ret, error; > - int retry = IST3038C_I2C_RETRY_COUNT; > + int retry = IST30XXC_I2C_RETRY_COUNT; > > /* Retry in case the controller fails to respond */ > do { > @@ -84,7 +85,7 @@ static irqreturn_t imagis_interrupt(int irq, void *dev_id) > int i; > int error; > > - error = imagis_i2c_read_reg(ts, IST3038C_REG_INTR_MESSAGE, > + error = imagis_i2c_read_reg(ts, IST30XXC_REG_INTR_MESSAGE, > &intr_message); > if (error) { > dev_err(&ts->client->dev, > @@ -92,20 +93,20 @@ static irqreturn_t imagis_interrupt(int irq, void *dev_id) > goto out; > } > > - finger_count = (intr_message & IST3038C_FINGER_COUNT_MASK) >> > - IST3038C_FINGER_COUNT_SHIFT; > - if (finger_count > IST3038C_MAX_FINGER_NUM) { > + finger_count = (intr_message & IST30XXC_FINGER_COUNT_MASK) >> > + IST30XXC_FINGER_COUNT_SHIFT; > + if (finger_count > IST30XXC_MAX_FINGER_NUM) { > dev_err(&ts->client->dev, > "finger count %d is more than maximum supported\n", > finger_count); > goto out; > } > > - finger_pressed = intr_message & IST3038C_FINGER_STATUS_MASK; > + finger_pressed = intr_message & IST30XXC_FINGER_STATUS_MASK; > > for (i = 0; i < finger_count; i++) { > error = imagis_i2c_read_reg(ts, > - IST3038C_REG_TOUCH_COORD + (i * 4), > + IST30XXC_REG_TOUCH_COORD + (i * 4), > &finger_status); > if (error) { > dev_err(&ts->client->dev, > @@ -118,12 +119,12 @@ static irqreturn_t imagis_interrupt(int irq, void *dev_id) > input_mt_report_slot_state(ts->input_dev, MT_TOOL_FINGER, > finger_pressed & BIT(i)); > touchscreen_report_pos(ts->input_dev, &ts->prop, > - (finger_status & IST3038C_X_MASK) >> > - IST3038C_X_SHIFT, > - finger_status & IST3038C_Y_MASK, 1); > + (finger_status & IST30XXC_X_MASK) >> > + IST30XXC_X_SHIFT, > + finger_status & IST30XXC_Y_MASK, 1); > input_report_abs(ts->input_dev, ABS_MT_TOUCH_MAJOR, > - (finger_status & IST3038C_AREA_MASK) >> > - IST3038C_AREA_SHIFT); > + (finger_status & IST30XXC_AREA_MASK) >> > + IST30XXC_AREA_SHIFT); > } > > input_mt_sync_frame(ts->input_dev); > @@ -148,7 +149,7 @@ static int imagis_power_on(struct imagis_ts *ts) > if (error) > return error; > > - msleep(IST3038C_CHIP_ON_DELAY_MS); > + msleep(IST30XXC_CHIP_ON_DELAY_MS); > > return 0; > } > @@ -220,7 +221,7 @@ static int imagis_init_input_dev(struct imagis_ts *ts) > } > > error = input_mt_init_slots(input_dev, > - IST3038C_MAX_FINGER_NUM, > + IST30XXC_MAX_FINGER_NUM, > INPUT_MT_DIRECT | INPUT_MT_DROP_UNUSED); > if (error) { > dev_err(&ts->client->dev, > @@ -253,7 +254,7 @@ static int imagis_probe(struct i2c_client *i2c) > { > struct device *dev = &i2c->dev; > struct imagis_ts *ts; > - int chip_id, error; > + int chip_id, dt_chip_id, error; > > ts = devm_kzalloc(dev, sizeof(*ts), GFP_KERNEL); > if (!ts) > @@ -261,6 +262,8 @@ static int imagis_probe(struct i2c_client *i2c) > > ts->client = i2c; > > + dt_chip_id = (int)(uintptr_t)device_get_match_data(&i2c->dev); > + > error = imagis_init_regulators(ts); > if (error) { > dev_err(dev, "regulator init error: %d\n", error); > @@ -280,15 +283,15 @@ static int imagis_probe(struct i2c_client *i2c) > } > > error = imagis_i2c_read_reg(ts, > - IST3038C_REG_CHIPID | IST3038C_DIRECT_ACCESS, > + IST30XXC_REG_CHIPID | IST30XXC_DIRECT_ACCESS, > &chip_id); > if (error) { > dev_err(dev, "chip ID read failure: %d\n", error); > return error; > } > > - if (chip_id != IST3038C_WHOAMI) { > - dev_err(dev, "unknown chip ID: 0x%x\n", chip_id); > + if (chip_id != dt_chip_id) { > + dev_err(dev, "unknown or misconfigured chip ID: 0x%x\n", chip_id); > return -EINVAL; > } > > @@ -345,12 +348,18 @@ static DEFINE_SIMPLE_DEV_PM_OPS(imagis_pm_ops, imagis_suspend, imagis_resume); > > #ifdef CONFIG_OF > static const struct of_device_id imagis_of_match[] = { > - { .compatible = "imagis,ist3038c", }, > + { .compatible = "imagis,ist3038c", .data = (void *)IST3038C_WHOAMI, }, > { }, > }; > MODULE_DEVICE_TABLE(of, imagis_of_match); > #endif > > +static const struct i2c_device_id imagis_ts_i2c_id[] = { > + { "ist3038c", IST3038C_WHOAMI, }, > + { }, > +}; > +MODULE_DEVICE_TABLE(i2c, imagis_ts_i2c_id); > + > static struct i2c_driver imagis_ts_driver = { > .driver = { > .name = "imagis-touchscreen", @@ -358,10 +367,11 @@ static struct i2c_driver imagis_ts_driver = { > .of_match_table = of_match_ptr(imagis_of_match), }, .probe = > imagis_probe, + .id_table = imagis_ts_i2c_id, }; > module_i2c_driver(imagis_ts_driver); -MODULE_DESCRIPTION("Imagis IST3038C Touchscreen Driver"); > +MODULE_DESCRIPTION("Imagis IST30XXC Touchscreen Driver"); > MODULE_AUTHOR("Markuss Broks<markuss.broks@gmail.com>"); > MODULE_LICENSE("GPL"); Additionally, there used to be my series [1] that generalized the driver, but it never got applied. I will re-send it. It introduces `struct imagis_properties`, which contains register addresses for the registers that we use to read the touch input. In your specific case, I believe it should be: static const struct imagis_properties imagis_3032c_data = { .interrupt_msg_cmd = IST3038C_REG_INTR_MESSAGE, .touch_coord_cmd = IST3038C_REG_TOUCH_COORD, .whoami_cmd = IST3038C_REG_CHIPID, .whoami_val = IST3032C_WHOAMI, /* change it to your whoami */ }; As for the voltage set, I believe this does not belong in a kernel driver. You should set it in device-tree with `regulator-max-microvolt` and `regulator-min-microvolt`. Yours, - Markuss [1]: https://lore.kernel.org/lkml/20220504152406.8730-1-markuss.broks@gmail.com/ ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] input: generalize the Imagis touchscreen driver 2023-09-27 15:36 ` [PATCH 1/2] input: generalize the Imagis touchscreen driver Markuss Broks @ 2023-09-28 18:07 ` Karel Balej 2023-09-28 20:22 ` Markuss Broks 0 siblings, 1 reply; 8+ messages in thread From: Karel Balej @ 2023-09-28 18:07 UTC (permalink / raw) To: Markuss Broks, Karel Balej Cc: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-input, devicetree, linux-kernel, Duje Mihanović, ~postmarketos/upstreaming, Jeff LaBundy, linmengbo0689 Hello, Markuss, thank you for your response and please forgive that my original emails didn't reach you - I am trying to see if I can get the SMTP server I use for my primary address officially authorized to send emails in its name so that Google does not reject them. > To be fair, this driver should work with all Imagis3 chips, which could > be a better name for it. However, I agree with Jeff here: the driver > doesn't necessarily need renaming. I will be sure to drop the renaming for v2. > Additionally, there used to be my series [1] that generalized the > driver, but it never got applied. I will re-send it. It introduces > `struct imagis_properties`, which contains register addresses for the > registers that we use to read the touch input. In your specific case, I > believe it should be: > > static const struct imagis_properties imagis_3032c_data = > { > .interrupt_msg_cmd = IST3038C_REG_INTR_MESSAGE, > .touch_coord_cmd = IST3038C_REG_TOUCH_COORD, > .whoami_cmd = IST3038C_REG_CHIPID, > .whoami_val = IST3032C_WHOAMI, /* change it to your whoami */ > }; I have come across your patch series and in fact my original experiments were based on it. However I thought it was abandoned and decided not to try to make any further generalizations to the driver for now, except making it work with IST3032C. Should I then perhaps wait before your series gets applied before sending v2 of my changes? Or should I send you a patch on top of your series, where I would add the `struct imagis_properties` for the IST3032C (which you surely could do yourself, but I can at least test it with my device). Please let me know if and how we should coordinate. > As for the voltage set, I believe this does not belong in a kernel > driver. You should set it in device-tree with `regulator-max-microvolt` > and `regulator-min-microvolt`. Please see my response to Jeff regarding this. I will be happy to hear your thoughts on what I propose. Kind regards, K. B. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] input: generalize the Imagis touchscreen driver 2023-09-28 18:07 ` Karel Balej @ 2023-09-28 20:22 ` Markuss Broks 2023-09-29 16:37 ` Karel Balej 0 siblings, 1 reply; 8+ messages in thread From: Markuss Broks @ 2023-09-28 20:22 UTC (permalink / raw) To: Karel Balej, Karel Balej Cc: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-input, devicetree, linux-kernel, Duje Mihanović, ~postmarketos/upstreaming, Jeff LaBundy, linmengbo0689 Hi Karel, On 9/28/23 21:07, Karel Balej wrote: > Hello, Markuss, > > thank you for your response and please forgive that my original emails > didn't reach you - I am trying to see if I can get the SMTP server I use > for my primary address officially authorized to send emails in its name > so that Google does not reject them. Yeah, I have not received your first series, only knew of it when the replies came. It's fine though :) > >> To be fair, this driver should work with all Imagis3 chips, which could >> be a better name for it. However, I agree with Jeff here: the driver >> doesn't necessarily need renaming. > I will be sure to drop the renaming for v2. > >> Additionally, there used to be my series [1] that generalized the >> driver, but it never got applied. I will re-send it. It introduces >> `struct imagis_properties`, which contains register addresses for the >> registers that we use to read the touch input. In your specific case, I >> believe it should be: >> >> static const struct imagis_properties imagis_3032c_data = >> { >> .interrupt_msg_cmd = IST3038C_REG_INTR_MESSAGE, >> .touch_coord_cmd = IST3038C_REG_TOUCH_COORD, >> .whoami_cmd = IST3038C_REG_CHIPID, >> .whoami_val = IST3032C_WHOAMI, /* change it to your whoami */ >> }; > I have come across your patch series and in fact my original experiments > were based on it. However I thought it was abandoned and decided not to > try to make any further generalizations to the driver for now, except > making it work with IST3032C. Should I then perhaps wait before your > series gets applied before sending v2 of my changes? Or should I send > you a patch on top of your series, where I would add the `struct > imagis_properties` for the IST3032C (which you surely could do yourself, > but I can at least test it with my device). Please let me know if and > how we should coordinate. If you don't mind the extra hassle, I'm all in for my generalization thing going together with your series. Alternatively, I can resend it myself, but I believe it would be better if they go in bulk since they need to be applied together. > >> As for the voltage set, I believe this does not belong in a kernel >> driver. You should set it in device-tree with `regulator-max-microvolt` >> and `regulator-min-microvolt`. > Please see my response to Jeff regarding this. I will be happy to hear > your thoughts on what I propose. Actually, the regulator values belong to the device-tree, because the device-tree for the board is what describes the board's regulators, and since you know what components are installed on that specific board you can know which regulator values are supposed to be set for it. This manual voltage setting can cause conflicts with other drivers for example. Also some device can use a variable wide voltage range, and some only specific narrow one, and e.g. the driver with wide range would set it to voltage that isn't suitable for the narrow range device, so it's much better to just specify it manually than have it resolved. The actual min/max values for regulators or its voltage table is provided by the regulator driver itself, so there's not much point in specifying those again in the device-tree. > > Kind regards, > K. B. - Markuss ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] input: generalize the Imagis touchscreen driver 2023-09-28 20:22 ` Markuss Broks @ 2023-09-29 16:37 ` Karel Balej 2023-09-30 16:10 ` Markuss Broks 0 siblings, 1 reply; 8+ messages in thread From: Karel Balej @ 2023-09-29 16:37 UTC (permalink / raw) To: Markuss Broks, Karel Balej Cc: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-input, devicetree, linux-kernel, Duje Mihanović, ~postmarketos/upstreaming, Jeff LaBundy, linmengbo0689 Hello, Markuss, > If you don't mind the extra hassle, I'm all in for my generalization > thing going together with your series. > > Alternatively, I can resend it myself, but I believe it would be better > if they go in bulk since they need to be applied together. great. Do you wish to make any changes to your original series? If not, please let me know and I will use the v2 [1] as it is. > >> As for the voltage set, I believe this does not belong in a kernel > >> driver. You should set it in device-tree with `regulator-max-microvolt` > >> and `regulator-min-microvolt`. > > Please see my response to Jeff regarding this. I will be happy to hear > > your thoughts on what I propose. > > Actually, the regulator values belong to the device-tree, because the > device-tree for the board is what describes the board's regulators, and > since you know what components are installed on that specific board you > can know which regulator values are supposed to be set for it. > > [...] > > The actual min/max values for regulators or its voltage table is > provided by the regulator driver itself, so there's not much point in > specifying those again in the device-tree. I see. I think the reason why I thought what I wrote before is that downstream the regulator DTS lives separately from the board as a .dtsi file which made me think that it can be used universally. So if I understand correctly now, the hardware specifications of the regulator, such as the minimal and maximal voltage should be part of the driver, while the DT should contain requirements for the given use of the regulator (with a specific board) - is this correct? > This manual voltage setting can cause conflicts with other drivers for > example. Also some device can use a variable wide voltage range, and > some only specific narrow one, and e.g. the driver with wide range > would set it to voltage that isn't suitable for the narrow range > device, so it's much better to just specify it manually than have it > resolved. I would expect that in the case you describe, the kernel would set the voltage to a value which would satisfy both the ranges. I don't know what would happen if that was not possible (i. e. there was no intersection in the two requested voltage ranges), though. Or does a call to regulator_set_voltage set the voltage immediately taking notice only of the hardware contraints? I think I am having trouble understanding what this quote from the regulator_set_voltage documentation means: > If the regulator is shared between several devices then the lowest > request voltage that meets the system constraints will be used. But actually, it probably doesn't make sense that the kernel would try to resolve a range suitable for all calls to this function as, I assume, a single driver could call it multiple times with disjoint voltage intervals. Thank you for your patience. Kind regards, K. B. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] input: generalize the Imagis touchscreen driver 2023-09-29 16:37 ` Karel Balej @ 2023-09-30 16:10 ` Markuss Broks 0 siblings, 0 replies; 8+ messages in thread From: Markuss Broks @ 2023-09-30 16:10 UTC (permalink / raw) To: Karel Balej, Karel Balej Cc: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-input, devicetree, linux-kernel, Duje Mihanović, ~postmarketos/upstreaming, Jeff LaBundy, linmengbo0689 Hi Karel, On 9/29/23 19:37, Karel Balej wrote: > Hello, Markuss, > >> If you don't mind the extra hassle, I'm all in for my generalization >> thing going together with your series. >> >> Alternatively, I can resend it myself, but I believe it would be better >> if they go in bulk since they need to be applied together. > great. Do you wish to make any changes to your original series? If not, > please let me know and I will use the v2 [1] as it is. I believe that version should be fine. > >>>> As for the voltage set, I believe this does not belong in a kernel >>>> driver. You should set it in device-tree with `regulator-max-microvolt` >>>> and `regulator-min-microvolt`. >>> Please see my response to Jeff regarding this. I will be happy to hear >>> your thoughts on what I propose. >> Actually, the regulator values belong to the device-tree, because the >> device-tree for the board is what describes the board's regulators, and >> since you know what components are installed on that specific board you >> can know which regulator values are supposed to be set for it. >> >> [...] >> >> The actual min/max values for regulators or its voltage table is >> provided by the regulator driver itself, so there's not much point in >> specifying those again in the device-tree. > I see. I think the reason why I thought what I wrote before is that > downstream the regulator DTS lives separately from the board as a .dtsi > file which made me think that it can be used universally. So if I > understand correctly now, the hardware specifications of the regulator, > such as the minimal and maximal voltage should be part of the driver, > while the DT should contain requirements for the given use of the > regulator (with a specific board) - is this correct? That is correct. > >> This manual voltage setting can cause conflicts with other drivers for >> example. Also some device can use a variable wide voltage range, and >> some only specific narrow one, and e.g. the driver with wide range >> would set it to voltage that isn't suitable for the narrow range >> device, so it's much better to just specify it manually than have it >> resolved. > I would expect that in the case you describe, the kernel would set the > voltage to a value which would satisfy both the ranges. I don't know > what would happen if that was not possible (i. e. there was no > intersection in the two requested voltage ranges), though. Or does a > call to regulator_set_voltage set the voltage immediately taking notice > only of the hardware contraints? I think I am having trouble > understanding what this quote from the regulator_set_voltage > documentation means: > >> If the regulator is shared between several devices then the lowest >> request voltage that meets the system constraints will be used. > But actually, it probably doesn't make sense that the kernel would try > to resolve a range suitable for all calls to this function as, I assume, > a single driver could call it multiple times with disjoint voltage > intervals. Well, I assume kernel has some sort of behaviour for that stuff, but I believe setting the voltage by the device manually is discouraged in general. It should be possible, though. > > Thank you for your patience. > > Kind regards, > K. B. - Markuss ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 0/2] input: Imagis: add support for the IST3032C touchscreen @ 2023-09-26 17:35 Karel Balej 2023-09-26 17:35 ` [PATCH 1/2] input: generalize the Imagis touchscreen driver Karel Balej 0 siblings, 1 reply; 8+ messages in thread From: Karel Balej @ 2023-09-26 17:35 UTC (permalink / raw) To: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Markuss Broks, linux-input, devicetree, linux-kernel, Duje Mihanović, ~postmarketos/upstreaming Cc: Karel Balej This patch series extends the Imagis driver to support the IST3032C touchscreen, which is used for instance with the samsung,coreprimevelte smartphone, with which this was tested. To use it with this model however, the regulator driver needs to be ported first. Support for this smartphone is not yet in-tree, upstreaming is ongoing at [1]. [1] https://lore.kernel.org/all/20230812-pxa1908-lkml-v5-0-a5d51937ee34@skole.hr/ Karel Balej (2): input: generalize the Imagis touchscreen driver input: Imagis: add support for the IST3032C touchscreen ...gis,ist3038c.yaml => imagis,ist30xxc.yaml} | 3 +- MAINTAINERS | 2 +- drivers/input/touchscreen/Kconfig | 4 +- drivers/input/touchscreen/imagis.c | 99 ++++++++++++------- 4 files changed, 66 insertions(+), 42 deletions(-) rename Documentation/devicetree/bindings/input/touchscreen/{imagis,ist3038c.yaml => imagis,ist30xxc.yaml} (97%) -- 2.42.0 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/2] input: generalize the Imagis touchscreen driver 2023-09-26 17:35 [PATCH 0/2] input: Imagis: add support for the IST3032C touchscreen Karel Balej @ 2023-09-26 17:35 ` Karel Balej 2023-09-26 22:17 ` Conor Dooley 2023-09-27 1:48 ` Jeff LaBundy 0 siblings, 2 replies; 8+ messages in thread From: Karel Balej @ 2023-09-26 17:35 UTC (permalink / raw) To: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Markuss Broks, linux-input, devicetree, linux-kernel, Duje Mihanović, ~postmarketos/upstreaming Cc: Karel Balej This driver should work with other Imagis ICs of the IST30**C series. Make that more apparent. Co-developed-by: Duje Mihanović <duje.mihanovic@skole.hr> Signed-off-by: Duje Mihanović <duje.mihanovic@skole.hr> Signed-off-by: Karel Balej <balejk@matfyz.cz> --- ...gis,ist3038c.yaml => imagis,ist30xxc.yaml} | 2 +- MAINTAINERS | 2 +- drivers/input/touchscreen/Kconfig | 4 +- drivers/input/touchscreen/imagis.c | 86 +++++++++++-------- 4 files changed, 52 insertions(+), 42 deletions(-) rename Documentation/devicetree/bindings/input/touchscreen/{imagis,ist3038c.yaml => imagis,ist30xxc.yaml} (99%) diff --git a/Documentation/devicetree/bindings/input/touchscreen/imagis,ist3038c.yaml b/Documentation/devicetree/bindings/input/touchscreen/imagis,ist30xxc.yaml similarity index 99% rename from Documentation/devicetree/bindings/input/touchscreen/imagis,ist3038c.yaml rename to Documentation/devicetree/bindings/input/touchscreen/imagis,ist30xxc.yaml index 0d6b033fd5fb..09bf3a6acc5e 100644 --- a/Documentation/devicetree/bindings/input/touchscreen/imagis,ist3038c.yaml +++ b/Documentation/devicetree/bindings/input/touchscreen/imagis,ist30xxc.yaml @@ -1,7 +1,7 @@ # SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML 1.2 --- -$id: http://devicetree.org/schemas/input/touchscreen/imagis,ist3038c.yaml# +$id: http://devicetree.org/schemas/input/touchscreen/imagis,ist30xxc.yaml# $schema: http://devicetree.org/meta-schemas/core.yaml# title: Imagis IST30XXC family touchscreen controller diff --git a/MAINTAINERS b/MAINTAINERS index b19995690904..b23e76418d94 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -10209,7 +10209,7 @@ F: drivers/usb/atm/ueagle-atm.c IMAGIS TOUCHSCREEN DRIVER M: Markuss Broks <markuss.broks@gmail.com> S: Maintained -F: Documentation/devicetree/bindings/input/touchscreen/imagis,ist3038c.yaml +F: Documentation/devicetree/bindings/input/touchscreen/imagis,ist30xxc.yaml F: drivers/input/touchscreen/imagis.c IMGTEC ASCII LCD DRIVER diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig index e3e2324547b9..45503aa2653e 100644 --- a/drivers/input/touchscreen/Kconfig +++ b/drivers/input/touchscreen/Kconfig @@ -665,10 +665,10 @@ config TOUCHSCREEN_NOVATEK_NVT_TS module will be called novatek-nvt-ts. config TOUCHSCREEN_IMAGIS - tristate "Imagis touchscreen support" + tristate "Imagis IST30XXC touchscreen support" depends on I2C help - Say Y here if you have an Imagis IST30xxC touchscreen. + Say Y here if you have an Imagis IST30XXC touchscreen. If unsure, say N. To compile this driver as a module, choose M here: the diff --git a/drivers/input/touchscreen/imagis.c b/drivers/input/touchscreen/imagis.c index 07111ca24455..4456f1b4d527 100644 --- a/drivers/input/touchscreen/imagis.c +++ b/drivers/input/touchscreen/imagis.c @@ -11,25 +11,26 @@ #include <linux/property.h> #include <linux/regulator/consumer.h> -#define IST3038C_HIB_ACCESS (0x800B << 16) -#define IST3038C_DIRECT_ACCESS BIT(31) -#define IST3038C_REG_CHIPID 0x40001000 -#define IST3038C_REG_HIB_BASE 0x30000100 -#define IST3038C_REG_TOUCH_STATUS (IST3038C_REG_HIB_BASE | IST3038C_HIB_ACCESS) -#define IST3038C_REG_TOUCH_COORD (IST3038C_REG_HIB_BASE | IST3038C_HIB_ACCESS | 0x8) -#define IST3038C_REG_INTR_MESSAGE (IST3038C_REG_HIB_BASE | IST3038C_HIB_ACCESS | 0x4) +#define IST30XXC_HIB_ACCESS (0x800B << 16) +#define IST30XXC_DIRECT_ACCESS BIT(31) +#define IST30XXC_REG_CHIPID 0x40001000 +#define IST30XXC_REG_HIB_BASE 0x30000100 +#define IST30XXC_REG_TOUCH_STATUS (IST30XXC_REG_HIB_BASE | IST30XXC_HIB_ACCESS) +#define IST30XXC_REG_TOUCH_COORD (IST30XXC_REG_HIB_BASE | IST30XXC_HIB_ACCESS | 0x8) +#define IST30XXC_REG_INTR_MESSAGE (IST30XXC_REG_HIB_BASE | IST30XXC_HIB_ACCESS | 0x4) +#define IST30XXC_CHIP_ON_DELAY_MS 60 +#define IST30XXC_I2C_RETRY_COUNT 3 +#define IST30XXC_MAX_FINGER_NUM 10 +#define IST30XXC_X_MASK GENMASK(23, 12) +#define IST30XXC_X_SHIFT 12 +#define IST30XXC_Y_MASK GENMASK(11, 0) +#define IST30XXC_AREA_MASK GENMASK(27, 24) +#define IST30XXC_AREA_SHIFT 24 +#define IST30XXC_FINGER_COUNT_MASK GENMASK(15, 12) +#define IST30XXC_FINGER_COUNT_SHIFT 12 +#define IST30XXC_FINGER_STATUS_MASK GENMASK(9, 0) + #define IST3038C_WHOAMI 0x38c -#define IST3038C_CHIP_ON_DELAY_MS 60 -#define IST3038C_I2C_RETRY_COUNT 3 -#define IST3038C_MAX_FINGER_NUM 10 -#define IST3038C_X_MASK GENMASK(23, 12) -#define IST3038C_X_SHIFT 12 -#define IST3038C_Y_MASK GENMASK(11, 0) -#define IST3038C_AREA_MASK GENMASK(27, 24) -#define IST3038C_AREA_SHIFT 24 -#define IST3038C_FINGER_COUNT_MASK GENMASK(15, 12) -#define IST3038C_FINGER_COUNT_SHIFT 12 -#define IST3038C_FINGER_STATUS_MASK GENMASK(9, 0) struct imagis_ts { struct i2c_client *client; @@ -57,7 +58,7 @@ static int imagis_i2c_read_reg(struct imagis_ts *ts, }, }; int ret, error; - int retry = IST3038C_I2C_RETRY_COUNT; + int retry = IST30XXC_I2C_RETRY_COUNT; /* Retry in case the controller fails to respond */ do { @@ -84,7 +85,7 @@ static irqreturn_t imagis_interrupt(int irq, void *dev_id) int i; int error; - error = imagis_i2c_read_reg(ts, IST3038C_REG_INTR_MESSAGE, + error = imagis_i2c_read_reg(ts, IST30XXC_REG_INTR_MESSAGE, &intr_message); if (error) { dev_err(&ts->client->dev, @@ -92,20 +93,20 @@ static irqreturn_t imagis_interrupt(int irq, void *dev_id) goto out; } - finger_count = (intr_message & IST3038C_FINGER_COUNT_MASK) >> - IST3038C_FINGER_COUNT_SHIFT; - if (finger_count > IST3038C_MAX_FINGER_NUM) { + finger_count = (intr_message & IST30XXC_FINGER_COUNT_MASK) >> + IST30XXC_FINGER_COUNT_SHIFT; + if (finger_count > IST30XXC_MAX_FINGER_NUM) { dev_err(&ts->client->dev, "finger count %d is more than maximum supported\n", finger_count); goto out; } - finger_pressed = intr_message & IST3038C_FINGER_STATUS_MASK; + finger_pressed = intr_message & IST30XXC_FINGER_STATUS_MASK; for (i = 0; i < finger_count; i++) { error = imagis_i2c_read_reg(ts, - IST3038C_REG_TOUCH_COORD + (i * 4), + IST30XXC_REG_TOUCH_COORD + (i * 4), &finger_status); if (error) { dev_err(&ts->client->dev, @@ -118,12 +119,12 @@ static irqreturn_t imagis_interrupt(int irq, void *dev_id) input_mt_report_slot_state(ts->input_dev, MT_TOOL_FINGER, finger_pressed & BIT(i)); touchscreen_report_pos(ts->input_dev, &ts->prop, - (finger_status & IST3038C_X_MASK) >> - IST3038C_X_SHIFT, - finger_status & IST3038C_Y_MASK, 1); + (finger_status & IST30XXC_X_MASK) >> + IST30XXC_X_SHIFT, + finger_status & IST30XXC_Y_MASK, 1); input_report_abs(ts->input_dev, ABS_MT_TOUCH_MAJOR, - (finger_status & IST3038C_AREA_MASK) >> - IST3038C_AREA_SHIFT); + (finger_status & IST30XXC_AREA_MASK) >> + IST30XXC_AREA_SHIFT); } input_mt_sync_frame(ts->input_dev); @@ -148,7 +149,7 @@ static int imagis_power_on(struct imagis_ts *ts) if (error) return error; - msleep(IST3038C_CHIP_ON_DELAY_MS); + msleep(IST30XXC_CHIP_ON_DELAY_MS); return 0; } @@ -220,7 +221,7 @@ static int imagis_init_input_dev(struct imagis_ts *ts) } error = input_mt_init_slots(input_dev, - IST3038C_MAX_FINGER_NUM, + IST30XXC_MAX_FINGER_NUM, INPUT_MT_DIRECT | INPUT_MT_DROP_UNUSED); if (error) { dev_err(&ts->client->dev, @@ -253,7 +254,7 @@ static int imagis_probe(struct i2c_client *i2c) { struct device *dev = &i2c->dev; struct imagis_ts *ts; - int chip_id, error; + int chip_id, dt_chip_id, error; ts = devm_kzalloc(dev, sizeof(*ts), GFP_KERNEL); if (!ts) @@ -261,6 +262,8 @@ static int imagis_probe(struct i2c_client *i2c) ts->client = i2c; + dt_chip_id = (int)(uintptr_t)device_get_match_data(&i2c->dev); + error = imagis_init_regulators(ts); if (error) { dev_err(dev, "regulator init error: %d\n", error); @@ -280,15 +283,15 @@ static int imagis_probe(struct i2c_client *i2c) } error = imagis_i2c_read_reg(ts, - IST3038C_REG_CHIPID | IST3038C_DIRECT_ACCESS, + IST30XXC_REG_CHIPID | IST30XXC_DIRECT_ACCESS, &chip_id); if (error) { dev_err(dev, "chip ID read failure: %d\n", error); return error; } - if (chip_id != IST3038C_WHOAMI) { - dev_err(dev, "unknown chip ID: 0x%x\n", chip_id); + if (chip_id != dt_chip_id) { + dev_err(dev, "unknown or misconfigured chip ID: 0x%x\n", chip_id); return -EINVAL; } @@ -345,12 +348,18 @@ static DEFINE_SIMPLE_DEV_PM_OPS(imagis_pm_ops, imagis_suspend, imagis_resume); #ifdef CONFIG_OF static const struct of_device_id imagis_of_match[] = { - { .compatible = "imagis,ist3038c", }, + { .compatible = "imagis,ist3038c", .data = (void *)IST3038C_WHOAMI, }, { }, }; MODULE_DEVICE_TABLE(of, imagis_of_match); #endif +static const struct i2c_device_id imagis_ts_i2c_id[] = { + { "ist3038c", IST3038C_WHOAMI, }, + { }, +}; +MODULE_DEVICE_TABLE(i2c, imagis_ts_i2c_id); + static struct i2c_driver imagis_ts_driver = { .driver = { .name = "imagis-touchscreen", @@ -358,10 +367,11 @@ static struct i2c_driver imagis_ts_driver = { .of_match_table = of_match_ptr(imagis_of_match), }, .probe = imagis_probe, + .id_table = imagis_ts_i2c_id, }; module_i2c_driver(imagis_ts_driver); -MODULE_DESCRIPTION("Imagis IST3038C Touchscreen Driver"); +MODULE_DESCRIPTION("Imagis IST30XXC Touchscreen Driver"); MODULE_AUTHOR("Markuss Broks <markuss.broks@gmail.com>"); MODULE_LICENSE("GPL"); -- 2.42.0 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] input: generalize the Imagis touchscreen driver 2023-09-26 17:35 ` [PATCH 1/2] input: generalize the Imagis touchscreen driver Karel Balej @ 2023-09-26 22:17 ` Conor Dooley 2023-09-27 1:48 ` Jeff LaBundy 1 sibling, 0 replies; 8+ messages in thread From: Conor Dooley @ 2023-09-26 22:17 UTC (permalink / raw) To: Karel Balej Cc: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Markuss Broks, linux-input, devicetree, linux-kernel, Duje Mihanović, ~postmarketos/upstreaming [-- Attachment #1: Type: text/plain, Size: 1162 bytes --] On Tue, Sep 26, 2023 at 07:35:23PM +0200, Karel Balej wrote: > This driver should work with other Imagis ICs of the IST30**C series. > Make that more apparent. > > Co-developed-by: Duje Mihanović <duje.mihanovic@skole.hr> > Signed-off-by: Duje Mihanović <duje.mihanovic@skole.hr> > Signed-off-by: Karel Balej <balejk@matfyz.cz> > --- > ...gis,ist3038c.yaml => imagis,ist30xxc.yaml} | 2 +- > MAINTAINERS | 2 +- > drivers/input/touchscreen/Kconfig | 4 +- > drivers/input/touchscreen/imagis.c | 86 +++++++++++-------- > 4 files changed, 52 insertions(+), 42 deletions(-) > rename Documentation/devicetree/bindings/input/touchscreen/{imagis,ist3038c.yaml => imagis,ist30xxc.yaml} (99%) > > diff --git a/Documentation/devicetree/bindings/input/touchscreen/imagis,ist3038c.yaml b/Documentation/devicetree/bindings/input/touchscreen/imagis,ist30xxc.yaml > similarity index 99% > rename from Documentation/devicetree/bindings/input/touchscreen/imagis,ist3038c.yaml > rename to Documentation/devicetree/bindings/input/touchscreen/imagis,ist30xxc.yaml This rename is pointless. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 228 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] input: generalize the Imagis touchscreen driver 2023-09-26 17:35 ` [PATCH 1/2] input: generalize the Imagis touchscreen driver Karel Balej 2023-09-26 22:17 ` Conor Dooley @ 2023-09-27 1:48 ` Jeff LaBundy 1 sibling, 0 replies; 8+ messages in thread From: Jeff LaBundy @ 2023-09-27 1:48 UTC (permalink / raw) To: Karel Balej Cc: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Markuss Broks, linux-input, devicetree, linux-kernel, Duje Mihanović, ~postmarketos/upstreaming Hi Karel, On Tue, Sep 26, 2023 at 07:35:23PM +0200, Karel Balej wrote: > This driver should work with other Imagis ICs of the IST30**C series. > Make that more apparent. > > Co-developed-by: Duje Mihanović <duje.mihanovic@skole.hr> > Signed-off-by: Duje Mihanović <duje.mihanovic@skole.hr> > Signed-off-by: Karel Balej <balejk@matfyz.cz> > --- > ...gis,ist3038c.yaml => imagis,ist30xxc.yaml} | 2 +- > MAINTAINERS | 2 +- > drivers/input/touchscreen/Kconfig | 4 +- > drivers/input/touchscreen/imagis.c | 86 +++++++++++-------- > 4 files changed, 52 insertions(+), 42 deletions(-) > rename Documentation/devicetree/bindings/input/touchscreen/{imagis,ist3038c.yaml => imagis,ist30xxc.yaml} (99%) > > diff --git a/Documentation/devicetree/bindings/input/touchscreen/imagis,ist3038c.yaml b/Documentation/devicetree/bindings/input/touchscreen/imagis,ist30xxc.yaml > similarity index 99% > rename from Documentation/devicetree/bindings/input/touchscreen/imagis,ist3038c.yaml > rename to Documentation/devicetree/bindings/input/touchscreen/imagis,ist30xxc.yaml > index 0d6b033fd5fb..09bf3a6acc5e 100644 > --- a/Documentation/devicetree/bindings/input/touchscreen/imagis,ist3038c.yaml > +++ b/Documentation/devicetree/bindings/input/touchscreen/imagis,ist30xxc.yaml > @@ -1,7 +1,7 @@ > # SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > %YAML 1.2 > --- > -$id: http://devicetree.org/schemas/input/touchscreen/imagis,ist3038c.yaml# > +$id: http://devicetree.org/schemas/input/touchscreen/imagis,ist30xxc.yaml# > $schema: http://devicetree.org/meta-schemas/core.yaml# > > title: Imagis IST30XXC family touchscreen controller > diff --git a/MAINTAINERS b/MAINTAINERS > index b19995690904..b23e76418d94 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -10209,7 +10209,7 @@ F: drivers/usb/atm/ueagle-atm.c > IMAGIS TOUCHSCREEN DRIVER > M: Markuss Broks <markuss.broks@gmail.com> > S: Maintained > -F: Documentation/devicetree/bindings/input/touchscreen/imagis,ist3038c.yaml > +F: Documentation/devicetree/bindings/input/touchscreen/imagis,ist30xxc.yaml > F: drivers/input/touchscreen/imagis.c > > IMGTEC ASCII LCD DRIVER > diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig > index e3e2324547b9..45503aa2653e 100644 > --- a/drivers/input/touchscreen/Kconfig > +++ b/drivers/input/touchscreen/Kconfig > @@ -665,10 +665,10 @@ config TOUCHSCREEN_NOVATEK_NVT_TS > module will be called novatek-nvt-ts. > > config TOUCHSCREEN_IMAGIS > - tristate "Imagis touchscreen support" > + tristate "Imagis IST30XXC touchscreen support" > depends on I2C > help > - Say Y here if you have an Imagis IST30xxC touchscreen. > + Say Y here if you have an Imagis IST30XXC touchscreen. > If unsure, say N. > > To compile this driver as a module, choose M here: the > diff --git a/drivers/input/touchscreen/imagis.c b/drivers/input/touchscreen/imagis.c > index 07111ca24455..4456f1b4d527 100644 > --- a/drivers/input/touchscreen/imagis.c > +++ b/drivers/input/touchscreen/imagis.c > @@ -11,25 +11,26 @@ > #include <linux/property.h> > #include <linux/regulator/consumer.h> > > -#define IST3038C_HIB_ACCESS (0x800B << 16) > -#define IST3038C_DIRECT_ACCESS BIT(31) > -#define IST3038C_REG_CHIPID 0x40001000 > -#define IST3038C_REG_HIB_BASE 0x30000100 > -#define IST3038C_REG_TOUCH_STATUS (IST3038C_REG_HIB_BASE | IST3038C_HIB_ACCESS) > -#define IST3038C_REG_TOUCH_COORD (IST3038C_REG_HIB_BASE | IST3038C_HIB_ACCESS | 0x8) > -#define IST3038C_REG_INTR_MESSAGE (IST3038C_REG_HIB_BASE | IST3038C_HIB_ACCESS | 0x4) > +#define IST30XXC_HIB_ACCESS (0x800B << 16) > +#define IST30XXC_DIRECT_ACCESS BIT(31) > +#define IST30XXC_REG_CHIPID 0x40001000 > +#define IST30XXC_REG_HIB_BASE 0x30000100 > +#define IST30XXC_REG_TOUCH_STATUS (IST30XXC_REG_HIB_BASE | IST30XXC_HIB_ACCESS) > +#define IST30XXC_REG_TOUCH_COORD (IST30XXC_REG_HIB_BASE | IST30XXC_HIB_ACCESS | 0x8) > +#define IST30XXC_REG_INTR_MESSAGE (IST30XXC_REG_HIB_BASE | IST30XXC_HIB_ACCESS | 0x4) > +#define IST30XXC_CHIP_ON_DELAY_MS 60 > +#define IST30XXC_I2C_RETRY_COUNT 3 > +#define IST30XXC_MAX_FINGER_NUM 10 > +#define IST30XXC_X_MASK GENMASK(23, 12) > +#define IST30XXC_X_SHIFT 12 > +#define IST30XXC_Y_MASK GENMASK(11, 0) > +#define IST30XXC_AREA_MASK GENMASK(27, 24) > +#define IST30XXC_AREA_SHIFT 24 > +#define IST30XXC_FINGER_COUNT_MASK GENMASK(15, 12) > +#define IST30XXC_FINGER_COUNT_SHIFT 12 > +#define IST30XXC_FINGER_STATUS_MASK GENMASK(9, 0) > + > #define IST3038C_WHOAMI 0x38c > -#define IST3038C_CHIP_ON_DELAY_MS 60 > -#define IST3038C_I2C_RETRY_COUNT 3 > -#define IST3038C_MAX_FINGER_NUM 10 > -#define IST3038C_X_MASK GENMASK(23, 12) > -#define IST3038C_X_SHIFT 12 > -#define IST3038C_Y_MASK GENMASK(11, 0) > -#define IST3038C_AREA_MASK GENMASK(27, 24) > -#define IST3038C_AREA_SHIFT 24 > -#define IST3038C_FINGER_COUNT_MASK GENMASK(15, 12) > -#define IST3038C_FINGER_COUNT_SHIFT 12 > -#define IST3038C_FINGER_STATUS_MASK GENMASK(9, 0) > There is no need to change all of the namespacing from IST3038C to IST30XXC; this is purely cosmetic and adds noise to the actual patch. It is perfectly acceptable for the driver to call itself IST3038C throughout while remaining compatible with other devices (e.g. IST3032C), in fact this flexibility is the intent of the compatible string in the first place. [...] > > -MODULE_DESCRIPTION("Imagis IST3038C Touchscreen Driver"); > +MODULE_DESCRIPTION("Imagis IST30XXC Touchscreen Driver"); > MODULE_AUTHOR("Markuss Broks <markuss.broks@gmail.com>"); > MODULE_LICENSE("GPL"); > -- > 2.42.0 > Kind regards, Jeff LaBundy ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2023-09-30 16:10 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- [not found] <7b9864bf-2aa6-4510-ad98-276fbfaadc30@gmail.com> 2023-09-27 15:36 ` [PATCH 1/2] input: generalize the Imagis touchscreen driver Markuss Broks 2023-09-28 18:07 ` Karel Balej 2023-09-28 20:22 ` Markuss Broks 2023-09-29 16:37 ` Karel Balej 2023-09-30 16:10 ` Markuss Broks 2023-09-26 17:35 [PATCH 0/2] input: Imagis: add support for the IST3032C touchscreen Karel Balej 2023-09-26 17:35 ` [PATCH 1/2] input: generalize the Imagis touchscreen driver Karel Balej 2023-09-26 22:17 ` Conor Dooley 2023-09-27 1:48 ` Jeff LaBundy
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).