+ Sekhar for the board-da850-evm questions.
Yegor Yefremov <yegorslists@xxxxxxxxxxxxxx> writes:
> Hi Dmitry,
>
> On Sat, Feb 25, 2017 at 8:08 PM, Dmitry Torokhov
> <dmitry.torokhov@xxxxxxxxx> wrote:
>> Hi Yegor,
>>
>> On Fri, Feb 24, 2017 at 04:42:25PM +0100, yegorslists@xxxxxxxxxxxxxx wrote:
>>> From: Yegor Yefremov <yegorslists@xxxxxxxxxxxxxx>
>>>
>>> This patch adds support for DT to tps6507x-ts driver.
>>>
>>> Signed-off-by: Yegor Yefremov <yegorslists@xxxxxxxxxxxxxx>
>>> ---
>>> drivers/input/touchscreen/tps6507x-ts.c | 70 ++++++++++++++++++++-------------
>>> 1 file changed, 42 insertions(+), 28 deletions(-)
>>>
>>> diff --git a/drivers/input/touchscreen/tps6507x-ts.c b/drivers/input/touchscreen/tps6507x-ts.c
>>> index 5bf1ec6..2d61220 100644
>>> --- a/drivers/input/touchscreen/tps6507x-ts.c
>>> +++ b/drivers/input/touchscreen/tps6507x-ts.c
>>> @@ -22,6 +22,8 @@
>>> #include <linux/mfd/tps6507x.h>
>>> #include <linux/input/tps6507x-ts.h>
>>> #include <linux/delay.h>
>>> +#include <linux/of.h>
>>> +#include <linux/of_device.h>
>>>
>>> #define TSC_DEFAULT_POLL_PERIOD 30 /* ms */
>>> #define TPS_DEFAULT_MIN_PRESSURE 0x30
>>> @@ -199,33 +201,52 @@ static void tps6507x_ts_poll(struct input_polled_dev *poll_dev)
>>> tps6507x_adc_standby(tsc);
>>> }
>>>
>>> +static void tsc_init_data(struct tps6507x_ts *tsc,
>>> + struct input_dev *input_dev)
>>> +{
>>> + struct device_node *node = tsc->dev->of_node;
>>> + const struct tps6507x_board *tps_board =
>>> + dev_get_platdata(tsc->dev);
>>> + const struct touchscreen_init_data *init_data = NULL;
>>> +
>>> + if (tps_board)
>>> + /*
>>> + * init_data points to array of touchscreen_init structure
>>> + * coming from the board-evm file.
>>> + */
>>> + init_data = tps_board->tps6507x_ts_init_data;
>>> +
>>> + if (node == NULL && init_data == NULL) {
>>> + tsc->poll_dev->poll_interval = TSC_DEFAULT_POLL_PERIOD;
>>> + tsc->min_pressure = TPS_DEFAULT_MIN_PRESSURE;
>>
>> I think you want to always init with defaults, then override with
>> property data.
>
> ACK
>
>>> + } else if (init_data) {
>>> + tsc->poll_dev->poll_interval = init_data->poll_period;
>>> + tsc->min_pressure = init_data->min_pressure;
>>> + input_dev->id.vendor = init_data->vendor;
>>> + input_dev->id.product = init_data->product;
>>> + input_dev->id.version = init_data->version;
>>> + } else if (node) {
>>> + of_property_read_u32(node, "ti,poll-period",
>>> + &tsc->poll_dev->poll_interval);
>>> + of_property_read_u16(node, "ti,min-pressure",
>>> + &tsc->min_pressure);
>>
>> Anything but u32 is better avoided in DT as it requires special
>> annotation which is easily forgotten.
>
> Will convert to 32-bit values.
>
>> I also would prefer if we switched to generic device properties and
>> retired the static init data.
>
> do you mean converting the driver to DT-only variant?
>
>>> + of_property_read_u16(node, "ti,vendor",
>>> + &input_dev->id.vendor);
>>> + of_property_read_u16(node, "ti,product",
>>> + &input_dev->id.product);
>>
>> Would we not know vendor and product from the compatible string?
>>
>>> + of_property_read_u16(node, "ti,version",
>>> + &input_dev->id.version);
>>
>> I wonder if anyone cares about this.
>
> I've looked at other touchscreen drivers and this structure seems to
> be filled mostly for the RS232 connected drivers. So I'd suggest just
> to leave vendor, product and version at 0 and not provide these values
> via DT. At least tslib and hence Qt is working without these values
> set.
>
> @Kevin: so far the only user of this touch driver is da850-evm board.
> Can we drop the values for the id structure? Btw. what else is needed
> to remove arch/arm/mach-davinci/board-da850-evm.c completely?
>
> Yegor
>
>>> + }
>>> +}
>>> +
>>> static int tps6507x_ts_probe(struct platform_device *pdev)
>>> {
>>> struct tps6507x_dev *tps6507x_dev = dev_get_drvdata(pdev->dev.parent);
>>> - const struct tps6507x_board *tps_board;
>>> - const struct touchscreen_init_data *init_data;
>>> struct tps6507x_ts *tsc;
>>> struct input_polled_dev *poll_dev;
>>> struct input_dev *input_dev;
>>> int error;
>>>
>>> - /*
>>> - * tps_board points to pmic related constants
>>> - * coming from the board-evm file.
>>> - */
>>> - tps_board = dev_get_platdata(tps6507x_dev->dev);
>>> - if (!tps_board) {
>>> - dev_err(tps6507x_dev->dev,
>>> - "Could not find tps6507x platform data\n");
>>> - return -ENODEV;
>>> - }
>>> -
>>> - /*
>>> - * init_data points to array of regulator_init structures
>>> - * coming from the board-evm file.
>>> - */
>>> - init_data = tps_board->tps6507x_ts_init_data;
>>> -
>>> tsc = devm_kzalloc(&pdev->dev, sizeof(struct tps6507x_ts), GFP_KERNEL);
>>> if (!tsc) {
>>> dev_err(tps6507x_dev->dev, "failed to allocate driver data\n");
>>> @@ -234,8 +255,6 @@ static int tps6507x_ts_probe(struct platform_device *pdev)
>>>
>>> tsc->mfd = tps6507x_dev;
>>> tsc->dev = tps6507x_dev->dev;
>>> - tsc->min_pressure = init_data ?
>>> - init_data->min_pressure : TPS_DEFAULT_MIN_PRESSURE;
>>>
>>> snprintf(tsc->phys, sizeof(tsc->phys),
>>> "%s/input0", dev_name(tsc->dev));
>>> @@ -251,8 +270,6 @@ static int tps6507x_ts_probe(struct platform_device *pdev)
>>>
>>> poll_dev->private = tsc;
>>> poll_dev->poll = tps6507x_ts_poll;
>>> - poll_dev->poll_interval = init_data ?
>>> - init_data->poll_period : TSC_DEFAULT_POLL_PERIOD;
>>>
>>> input_dev = poll_dev->input;
>>> input_dev->evbit[0] = BIT_MASK(EV_KEY) | BIT_MASK(EV_ABS);
>>> @@ -266,11 +283,8 @@ static int tps6507x_ts_probe(struct platform_device *pdev)
>>> input_dev->phys = tsc->phys;
>>> input_dev->dev.parent = tsc->dev;
>>> input_dev->id.bustype = BUS_I2C;
>>> - if (init_data) {
>>> - input_dev->id.vendor = init_data->vendor;
>>> - input_dev->id.product = init_data->product;
>>> - input_dev->id.version = init_data->version;
>>> - }
>>> +
>>> + tsc_init_data(tsc, input_dev);
>>>
>>> error = tps6507x_adc_standby(tsc);
>>> if (error)
>>> --
>>> 2.1.4
>>>
>>
>> Thanks.
>>
>> --
>> Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
