mbox series

[v2,0/3] Input: add Hycon HY46XX Touchscreen controller

Message ID 20210401230358.2468618-1-giulio.benetti@benettiengineering.com
Headers show
Series Input: add Hycon HY46XX Touchscreen controller | expand

Message

Giulio Benetti April 1, 2021, 11:03 p.m. UTC
This patchset adds Hycon vendor, HY46XX touchscreen controller driver
and its .yaml binding.

---
V1->V2:
* changed authorship and SoBs to @benettiengineering.com domain
* fixed vendor commit log according to Jonathan Neuschäfer's suggestion
* fixed hy46xx bindings according to Rob Herring's suggestions
* fixed hy46xx driver according to Dmitry Torokhov's suggestions
further details are listed in single patches
---

Giulio Benetti (3):
  dt-bindings: Add Hycon Technology vendor prefix
  dt-bindings: touchscreen: Add HY46XX bindings
  Input: add driver for the Hycon HY46XX touchpanel series

 .../input/touchscreen/hycon,hy46xx.yaml       | 120 ++++
 .../devicetree/bindings/vendor-prefixes.yaml  |   2 +
 MAINTAINERS                                   |   7 +
 drivers/input/touchscreen/Kconfig             |  12 +
 drivers/input/touchscreen/Makefile            |   1 +
 drivers/input/touchscreen/hycon-hy46xx.c      | 591 ++++++++++++++++++
 6 files changed, 733 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/input/touchscreen/hycon,hy46xx.yaml
 create mode 100644 drivers/input/touchscreen/hycon-hy46xx.c

Comments

Giulio Benetti April 2, 2021, 3:23 p.m. UTC | #1
Hi Jonathan,

On 4/2/21 10:59 AM, Jonathan Neuschäfer wrote:
> Hi,

> 

> a few remarks below.

> 

> On Fri, Apr 02, 2021 at 01:03:58AM +0200, Giulio Benetti wrote:

>> This patch adds support for Hycon HY46XX.

>>

>> Signed-off-by: Giulio Benetti <giulio.benetti@benettiengineering.com>

>> ---

>> V1->V2:

>> * removed proximity-sensor-switch property according to previous patch

>> As suggested by Dmitry Torokhov

>> * moved i2c communaction to regmap use

>> * added macro to avoid magic number

>> * removed cmd variable that could uninitiliazed since we're using regmap now

>> * removed useless byte masking

>> * removed useless struct hycon_hy46xx_i2c_chip_data

>> * used IRQF_ONESHOT only for isr

>> ---

> 

> 

>> +config TOUCHSCREEN_HYCON_HY46XX

>> +	tristate "Hycon hy46xx touchscreen support"

>> +	depends on I2C

>> +	help

>> +	  Say Y here if you have a touchscreen using Hycon hy46xx,

>> +	  or something similar enough.

> 

> The "something similar enough" part doesn't seem relevant, because the

> driver only lists HY46xx chips (in the compatible strings), and no chips

> that are similar enough to work with the driver, but have a different

> part number.


Right

>> +static void hycon_hy46xx_get_defaults(struct device *dev, struct hycon_hy46xx_data *tsdata)

>> +{

>> +	bool val_bool;

>> +	int error;

>> +	u32 val;

>> +

>> +	error = device_property_read_u32(dev, "threshold", &val);

> 

> This seems to follow the old version of the binding, where

> Hycon-specific properties didn't have the "hycon," prefix.

> Please check that the driver still works with a devicetree that follows

> the newest version of the binding.


Ah yes, I've forgotten it while changing in bindings.

>> +MODULE_AUTHOR("Giulio Benetti <giulio.benetti@micronovasrl.com>");

> 

> This is a different email address than you used in the DT binding. If

> this is intentional, no problem (Just letting you know, in case it's

> unintentional).


I've missed that

> 

> Thanks,

> Jonathan Neuschäfer

> 


Thank you!
Best regards
-- 
Giulio Benetti
Benetti Engineering sas