Re: [PATCH 20/51] Input: atmel_mxt_ts - Set default irqflags when there is no pdata
On Sat, May 24, 2014 at 8:41 AM, Nick Dyer wrote: > Yufeng Shen wrote: >> On Thu, May 22, 2014 at 10:29 AM, Nick Dyer wrote: >>> Dmitry Torokhov wrote: >> Make the irqflags default to be IRQF_TRIGGER_FALLING if no platform data >> is >> provided. I think if there is no platform data we should use 0 as IRQ falgs and assume that IRQ line is properly configured by the board code or via device tree. >>> >>> Benson/Yufeng - do you still have a requirement to probe without platform >>> data or device tree? I'm just merging in some changes to add device tree >>> support, and it would simplify things a bit if I can drop this patch. >> >> It has been working for quite a while for boards/devices that don't >> provide platform data. If we drop the default IRQ flags, sure we can add >> code for each board to configure the IRQ separately, but that's just >> adding extra work. Is there strong reason why we should not do the >> default setting in the driver if it is not already configured in >> platform data? > > OK, I will keep it in my tree for the moment, since you are using it. > > The reason I checked is that in general, I would like to be conservative > about what is pushed upstream, because it will need maintaining for a long > time. > > The other reason is that in fact Atmel recommend IRQF_TRIGGER_LOW for these > chips, not IRQF_TRIGGER_FALLING, so there is a bit of an inconsistency here. I think I chose IRQF_TRIGGER_FALLING is because when I do a search in the upstream code where platform is configured, the irq is always set to be IRQF_TRIGGER_FALLING so I was assuming IRQF_TRIGGER_FALLING is a safe bet. But you would definitely know better than me on this since the atmel chips that I have access to are quite limited. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 20/51] Input: atmel_mxt_ts - Set default irqflags when there is no pdata
On Mon, May 26, 2014 at 1:23 AM, Dmitry Torokhov wrote: > > On Fri, May 23, 2014 at 12:37:46PM -0400, Yufeng Shen wrote: > > On Thu, May 22, 2014 at 10:29 AM, Nick Dyer wrote: > > > > > > Dmitry Torokhov wrote: > > > > On Thu, Jul 18, 2013 at 07:17:44PM +0200, rydb...@euromail.se wrote: > > > >>> From: Yufeng Shen > > > >>> This is the preparation for supporting the code path when there is > > > >>> platform data provided and still boot the device into a sane state > > > >>> with backup NVRAM config. > > > >>> > > > >>> Make the irqflags default to be IRQF_TRIGGER_FALLING if no platform > > > >>> data is > > > >>> provided. > > > > > > > > I think if there is no platform data we should use 0 as IRQ falgs and > > > > assume that IRQ line is properly configured by the board code or via > > > > device tree. > > > > > > Beson/Yufeng - do you still have a requirement to probe without platform > > > data or device tree? I'm just merging in some changes to add device tree > > > support, and it would simplify things a bit if I can drop this patch. > > > > > > It has been working for quite a while for boards/devices that don't > > provide platform > > data. If we drop the default IRQ flags, sure we can add code for each > > board to configure > > the IRQ separately, but that's just adding extra work. Is there strong > > reason why we > > should not do the default setting in the driver if it is not already > > configured in platform > > data ? > > > I am not saying that board code needs to add platform data. I am saying > that the board code needs to set up interrupt properly (via > irq_set_irq_type() or similar) and then the driver can use 0 as irqflags > argument in request_irq() in absence of DT/platform data. > > Thanks. > So my argument is mainly based on that the existing code is working (boards that do not have platform data are relying on the driver to set the default irq), and change the default value would need extra work to setup the irq, say as you suggested through irq_set_irq_type(). I am no expert in irq so it could be true that your suggested way is indeed better. I am in favor of keeping this patch is only because it requires no extra change for existing code that are using it. > -- > Dmitry > -- > To unsubscribe from this list: send the line "unsubscribe linux-input" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 20/51] Input: atmel_mxt_ts - Set default irqflags when there is no pdata
On Mon, May 26, 2014 at 1:23 AM, Dmitry Torokhov dmitry.torok...@gmail.com wrote: On Fri, May 23, 2014 at 12:37:46PM -0400, Yufeng Shen wrote: On Thu, May 22, 2014 at 10:29 AM, Nick Dyer nick.d...@itdev.co.uk wrote: Dmitry Torokhov wrote: On Thu, Jul 18, 2013 at 07:17:44PM +0200, rydb...@euromail.se wrote: From: Yufeng Shen mile...@chromium.org This is the preparation for supporting the code path when there is platform data provided and still boot the device into a sane state with backup NVRAM config. Make the irqflags default to be IRQF_TRIGGER_FALLING if no platform data is provided. I think if there is no platform data we should use 0 as IRQ falgs and assume that IRQ line is properly configured by the board code or via device tree. Beson/Yufeng - do you still have a requirement to probe without platform data or device tree? I'm just merging in some changes to add device tree support, and it would simplify things a bit if I can drop this patch. It has been working for quite a while for boards/devices that don't provide platform data. If we drop the default IRQ flags, sure we can add code for each board to configure the IRQ separately, but that's just adding extra work. Is there strong reason why we should not do the default setting in the driver if it is not already configured in platform data ? I am not saying that board code needs to add platform data. I am saying that the board code needs to set up interrupt properly (via irq_set_irq_type() or similar) and then the driver can use 0 as irqflags argument in request_irq() in absence of DT/platform data. Thanks. So my argument is mainly based on that the existing code is working (boards that do not have platform data are relying on the driver to set the default irq), and change the default value would need extra work to setup the irq, say as you suggested through irq_set_irq_type(). I am no expert in irq so it could be true that your suggested way is indeed better. I am in favor of keeping this patch is only because it requires no extra change for existing code that are using it. -- Dmitry -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 20/51] Input: atmel_mxt_ts - Set default irqflags when there is no pdata
On Sat, May 24, 2014 at 8:41 AM, Nick Dyer nick.d...@itdev.co.uk wrote: Yufeng Shen wrote: On Thu, May 22, 2014 at 10:29 AM, Nick Dyer nick.d...@itdev.co.uk wrote: Dmitry Torokhov wrote: Make the irqflags default to be IRQF_TRIGGER_FALLING if no platform data is provided. I think if there is no platform data we should use 0 as IRQ falgs and assume that IRQ line is properly configured by the board code or via device tree. Benson/Yufeng - do you still have a requirement to probe without platform data or device tree? I'm just merging in some changes to add device tree support, and it would simplify things a bit if I can drop this patch. It has been working for quite a while for boards/devices that don't provide platform data. If we drop the default IRQ flags, sure we can add code for each board to configure the IRQ separately, but that's just adding extra work. Is there strong reason why we should not do the default setting in the driver if it is not already configured in platform data? OK, I will keep it in my tree for the moment, since you are using it. The reason I checked is that in general, I would like to be conservative about what is pushed upstream, because it will need maintaining for a long time. The other reason is that in fact Atmel recommend IRQF_TRIGGER_LOW for these chips, not IRQF_TRIGGER_FALLING, so there is a bit of an inconsistency here. I think I chose IRQF_TRIGGER_FALLING is because when I do a search in the upstream code where platform is configured, the irq is always set to be IRQF_TRIGGER_FALLING so I was assuming IRQF_TRIGGER_FALLING is a safe bet. But you would definitely know better than me on this since the atmel chips that I have access to are quite limited. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 20/51] Input: atmel_mxt_ts - Set default irqflags when there is no pdata
On Fri, May 23, 2014 at 12:37:46PM -0400, Yufeng Shen wrote: > On Thu, May 22, 2014 at 10:29 AM, Nick Dyer wrote: > > > > Dmitry Torokhov wrote: > > > On Thu, Jul 18, 2013 at 07:17:44PM +0200, rydb...@euromail.se wrote: > > >>> From: Yufeng Shen > > >>> This is the preparation for supporting the code path when there is > > >>> platform data provided and still boot the device into a sane state > > >>> with backup NVRAM config. > > >>> > > >>> Make the irqflags default to be IRQF_TRIGGER_FALLING if no platform > > >>> data is > > >>> provided. > > > > > > I think if there is no platform data we should use 0 as IRQ falgs and > > > assume that IRQ line is properly configured by the board code or via > > > device tree. > > > > Beson/Yufeng - do you still have a requirement to probe without platform > > data or device tree? I'm just merging in some changes to add device tree > > support, and it would simplify things a bit if I can drop this patch. > > > It has been working for quite a while for boards/devices that don't > provide platform > data. If we drop the default IRQ flags, sure we can add code for each > board to configure > the IRQ separately, but that's just adding extra work. Is there strong > reason why we > should not do the default setting in the driver if it is not already > configured in platform > data ? I am not saying that board code needs to add platform data. I am saying that the board code needs to set up interrupt properly (via irq_set_irq_type() or similar) and then the driver can use 0 as irqflags argument in request_irq() in absence of DT/platform data. Thanks. -- Dmitry -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 20/51] Input: atmel_mxt_ts - Set default irqflags when there is no pdata
On Fri, May 23, 2014 at 12:37:46PM -0400, Yufeng Shen wrote: On Thu, May 22, 2014 at 10:29 AM, Nick Dyer nick.d...@itdev.co.uk wrote: Dmitry Torokhov wrote: On Thu, Jul 18, 2013 at 07:17:44PM +0200, rydb...@euromail.se wrote: From: Yufeng Shen mile...@chromium.org This is the preparation for supporting the code path when there is platform data provided and still boot the device into a sane state with backup NVRAM config. Make the irqflags default to be IRQF_TRIGGER_FALLING if no platform data is provided. I think if there is no platform data we should use 0 as IRQ falgs and assume that IRQ line is properly configured by the board code or via device tree. Beson/Yufeng - do you still have a requirement to probe without platform data or device tree? I'm just merging in some changes to add device tree support, and it would simplify things a bit if I can drop this patch. It has been working for quite a while for boards/devices that don't provide platform data. If we drop the default IRQ flags, sure we can add code for each board to configure the IRQ separately, but that's just adding extra work. Is there strong reason why we should not do the default setting in the driver if it is not already configured in platform data ? I am not saying that board code needs to add platform data. I am saying that the board code needs to set up interrupt properly (via irq_set_irq_type() or similar) and then the driver can use 0 as irqflags argument in request_irq() in absence of DT/platform data. Thanks. -- Dmitry -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 20/51] Input: atmel_mxt_ts - Set default irqflags when there is no pdata
Yufeng Shen wrote: > On Thu, May 22, 2014 at 10:29 AM, Nick Dyer wrote: >> Dmitry Torokhov wrote: > Make the irqflags default to be IRQF_TRIGGER_FALLING if no platform data > is > provided. >>> >>> I think if there is no platform data we should use 0 as IRQ falgs and >>> assume that IRQ line is properly configured by the board code or via >>> device tree. >> >> Benson/Yufeng - do you still have a requirement to probe without platform >> data or device tree? I'm just merging in some changes to add device tree >> support, and it would simplify things a bit if I can drop this patch. > > It has been working for quite a while for boards/devices that don't > provide platform data. If we drop the default IRQ flags, sure we can add > code for each board to configure the IRQ separately, but that's just > adding extra work. Is there strong reason why we should not do the > default setting in the driver if it is not already configured in > platform data? OK, I will keep it in my tree for the moment, since you are using it. The reason I checked is that in general, I would like to be conservative about what is pushed upstream, because it will need maintaining for a long time. The other reason is that in fact Atmel recommend IRQF_TRIGGER_LOW for these chips, not IRQF_TRIGGER_FALLING, so there is a bit of an inconsistency here. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 20/51] Input: atmel_mxt_ts - Set default irqflags when there is no pdata
Yufeng Shen wrote: On Thu, May 22, 2014 at 10:29 AM, Nick Dyer nick.d...@itdev.co.uk wrote: Dmitry Torokhov wrote: Make the irqflags default to be IRQF_TRIGGER_FALLING if no platform data is provided. I think if there is no platform data we should use 0 as IRQ falgs and assume that IRQ line is properly configured by the board code or via device tree. Benson/Yufeng - do you still have a requirement to probe without platform data or device tree? I'm just merging in some changes to add device tree support, and it would simplify things a bit if I can drop this patch. It has been working for quite a while for boards/devices that don't provide platform data. If we drop the default IRQ flags, sure we can add code for each board to configure the IRQ separately, but that's just adding extra work. Is there strong reason why we should not do the default setting in the driver if it is not already configured in platform data? OK, I will keep it in my tree for the moment, since you are using it. The reason I checked is that in general, I would like to be conservative about what is pushed upstream, because it will need maintaining for a long time. The other reason is that in fact Atmel recommend IRQF_TRIGGER_LOW for these chips, not IRQF_TRIGGER_FALLING, so there is a bit of an inconsistency here. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 20/51] Input: atmel_mxt_ts - Set default irqflags when there is no pdata
On Thu, May 22, 2014 at 10:29 AM, Nick Dyer wrote: > > Dmitry Torokhov wrote: > > On Thu, Jul 18, 2013 at 07:17:44PM +0200, rydb...@euromail.se wrote: > >>> From: Yufeng Shen > >>> This is the preparation for supporting the code path when there is > >>> platform data provided and still boot the device into a sane state > >>> with backup NVRAM config. > >>> > >>> Make the irqflags default to be IRQF_TRIGGER_FALLING if no platform data > >>> is > >>> provided. > > > > I think if there is no platform data we should use 0 as IRQ falgs and > > assume that IRQ line is properly configured by the board code or via > > device tree. > > Beson/Yufeng - do you still have a requirement to probe without platform > data or device tree? I'm just merging in some changes to add device tree > support, and it would simplify things a bit if I can drop this patch. It has been working for quite a while for boards/devices that don't provide platform data. If we drop the default IRQ flags, sure we can add code for each board to configure the IRQ separately, but that's just adding extra work. Is there strong reason why we should not do the default setting in the driver if it is not already configured in platform data ? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 20/51] Input: atmel_mxt_ts - Set default irqflags when there is no pdata
On Thu, May 22, 2014 at 10:29 AM, Nick Dyer nick.d...@itdev.co.uk wrote: Dmitry Torokhov wrote: On Thu, Jul 18, 2013 at 07:17:44PM +0200, rydb...@euromail.se wrote: From: Yufeng Shen mile...@chromium.org This is the preparation for supporting the code path when there is platform data provided and still boot the device into a sane state with backup NVRAM config. Make the irqflags default to be IRQF_TRIGGER_FALLING if no platform data is provided. I think if there is no platform data we should use 0 as IRQ falgs and assume that IRQ line is properly configured by the board code or via device tree. Beson/Yufeng - do you still have a requirement to probe without platform data or device tree? I'm just merging in some changes to add device tree support, and it would simplify things a bit if I can drop this patch. It has been working for quite a while for boards/devices that don't provide platform data. If we drop the default IRQ flags, sure we can add code for each board to configure the IRQ separately, but that's just adding extra work. Is there strong reason why we should not do the default setting in the driver if it is not already configured in platform data ? -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 20/51] Input: atmel_mxt_ts - Set default irqflags when there is no pdata
Dmitry Torokhov wrote: > On Thu, Jul 18, 2013 at 07:17:44PM +0200, rydb...@euromail.se wrote: >>> From: Yufeng Shen >>> This is the preparation for supporting the code path when there is >>> platform data provided and still boot the device into a sane state >>> with backup NVRAM config. >>> >>> Make the irqflags default to be IRQF_TRIGGER_FALLING if no platform data is >>> provided. > > I think if there is no platform data we should use 0 as IRQ falgs and > assume that IRQ line is properly configured by the board code or via > device tree. Beson/Yufeng - do you still have a requirement to probe without platform data or device tree? I'm just merging in some changes to add device tree support, and it would simplify things a bit if I can drop this patch. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 20/51] Input: atmel_mxt_ts - Set default irqflags when there is no pdata
Dmitry Torokhov wrote: On Thu, Jul 18, 2013 at 07:17:44PM +0200, rydb...@euromail.se wrote: From: Yufeng Shen mile...@chromium.org This is the preparation for supporting the code path when there is platform data provided and still boot the device into a sane state with backup NVRAM config. Make the irqflags default to be IRQF_TRIGGER_FALLING if no platform data is provided. I think if there is no platform data we should use 0 as IRQ falgs and assume that IRQ line is properly configured by the board code or via device tree. Beson/Yufeng - do you still have a requirement to probe without platform data or device tree? I'm just merging in some changes to add device tree support, and it would simplify things a bit if I can drop this patch. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 20/51] Input: atmel_mxt_ts - Set default irqflags when there is no pdata
On Thu, Jul 18, 2013 at 07:17:44PM +0200, rydb...@euromail.se wrote: > Hi Nick, > > > From: Yufeng Shen > > > > This is the preparation for supporting the code path when there is > > platform data provided and still boot the device into a sane state > > with backup NVRAM config. > > > > Make the irqflags default to be IRQF_TRIGGER_FALLING if no platform data is > > provided. I think if there is no platform data we should use 0 as IRQ falgs and assume that IRQ line is properly configured by the board code or via device tree. Thanks. -- Dmitry -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 20/51] Input: atmel_mxt_ts - Set default irqflags when there is no pdata
On Thu, Jul 18, 2013 at 07:17:44PM +0200, rydb...@euromail.se wrote: Hi Nick, From: Yufeng Shen mile...@chromium.org This is the preparation for supporting the code path when there is platform data provided and still boot the device into a sane state with backup NVRAM config. Make the irqflags default to be IRQF_TRIGGER_FALLING if no platform data is provided. I think if there is no platform data we should use 0 as IRQ falgs and assume that IRQ line is properly configured by the board code or via device tree. Thanks. -- Dmitry -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 20/51] Input: atmel_mxt_ts - Set default irqflags when there is no pdata
Hi Nick,
> From: Yufeng Shen
>
> This is the preparation for supporting the code path when there is
> platform data provided and still boot the device into a sane state
> with backup NVRAM config.
>
> Make the irqflags default to be IRQF_TRIGGER_FALLING if no platform data is
> provided.
>
> Signed-off-by: Yufeng Shen
> Signed-off-by: Daniel Kurtz
> Signed-off-by: Nick Dyer
> Acked-by: Benson Leung
> ---
> drivers/input/touchscreen/atmel_mxt_ts.c | 54
> +-
> 1 file changed, 39 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c
> b/drivers/input/touchscreen/atmel_mxt_ts.c
> index 1334e5b..2645d36 100644
> --- a/drivers/input/touchscreen/atmel_mxt_ts.c
> +++ b/drivers/input/touchscreen/atmel_mxt_ts.c
> @@ -232,7 +232,7 @@ struct mxt_data {
> struct i2c_client *client;
> struct input_dev *input_dev;
> char phys[64]; /* device physical location */
> - const struct mxt_platform_data *pdata;
> + struct mxt_platform_data *pdata;
> struct mxt_object *object_table;
> struct mxt_info info;
> unsigned int irq;
> @@ -1640,10 +1640,29 @@ static void mxt_input_close(struct input_dev *dev)
> mxt_stop(data);
> }
>
> +static int mxt_handle_pdata(struct mxt_data *data)
> +{
> + data->pdata = dev_get_platdata(>client->dev);
> +
> + /* Use provided platform data if present */
> + if (data->pdata)
> + return 0;
> +
> + data->pdata = kzalloc(sizeof(*data->pdata), GFP_KERNEL);
> + if (!data->pdata) {
> + dev_err(>client->dev, "Failed to allocate pdata\n");
> + return -ENOMEM;
> + }
Any chance this could be a static instead?
> +
> + /* Set default parameters */
> + data->pdata->irqflags = IRQF_TRIGGER_FALLING;
> +
> + return 0;
> +}
> +
Opencode instead?
> static int mxt_probe(struct i2c_client *client,
> const struct i2c_device_id *id)
> {
> - const struct mxt_platform_data *pdata = client->dev.platform_data;
This line keeps reappearing in various versions of this
function. Perhaps it should simply stay as is instead?
> struct mxt_data *data;
> struct input_dev *input_dev;
> int error;
> @@ -1651,9 +1670,6 @@ static int mxt_probe(struct i2c_client *client,
> unsigned int mt_flags = 0;
> int i;
>
> - if (!pdata)
Why not just initialize the default here instead?
> - return -EINVAL;
> -
> data = kzalloc(sizeof(struct mxt_data), GFP_KERNEL);
> input_dev = input_allocate_device();
> if (!data || !input_dev) {
> @@ -1675,19 +1691,23 @@ static int mxt_probe(struct i2c_client *client,
>
> data->client = client;
> data->input_dev = input_dev;
> - data->pdata = pdata;
> data->irq = client->irq;
> + i2c_set_clientdata(client, data);
> +
> + error = mxt_handle_pdata(data);
> + if (error)
> + goto err_free_mem;
then this would go away
>
> init_completion(>bl_completion);
> init_completion(>reset_completion);
> init_completion(>crc_completion);
>
> - error = request_threaded_irq(client->irq, NULL, mxt_interrupt,
> - pdata->irqflags | IRQF_ONESHOT,
> + error = request_threaded_irq(data->irq, NULL, mxt_interrupt,
> + data->pdata->irqflags | IRQF_ONESHOT,
>client->name, data);
and this hunk
> if (error) {
> dev_err(>dev, "Failed to register interrupt\n");
> - goto err_free_mem;
> + goto err_free_pdata;
> }
>
> disable_irq(client->irq);
> @@ -1700,13 +1720,13 @@ static int mxt_probe(struct i2c_client *client,
> __set_bit(EV_KEY, input_dev->evbit);
> __set_bit(BTN_TOUCH, input_dev->keybit);
>
> - if (pdata->t19_num_keys) {
> + if (data->pdata->t19_num_keys) {
and this hunk
> __set_bit(INPUT_PROP_BUTTONPAD, input_dev->propbit);
>
> - for (i = 0; i < pdata->t19_num_keys; i++)
> - if (pdata->t19_keymap[i] != KEY_RESERVED)
> + for (i = 0; i < data->pdata->t19_num_keys; i++)
> + if (data->pdata->t19_keymap[i] != KEY_RESERVED)
> input_set_capability(input_dev, EV_KEY,
> - pdata->t19_keymap[i]);
> + data->pdata->t19_keymap[i]);
>
> mt_flags |= INPUT_MT_POINTER;
>
> @@ -1743,7 +1763,6 @@ static int mxt_probe(struct i2c_client *client,
>0, 255, 0, 0);
>
> input_set_drvdata(input_dev, data);
> - i2c_set_clientdata(client, data);
>
> error = input_register_device(input_dev);
> if (error) {
> @@ -1767,7 +1786,10 @@ err_unregister_device:
> err_free_object:
> kfree(data->object_table);
> err_free_irq:
> -
Re: [PATCH 20/51] Input: atmel_mxt_ts - Set default irqflags when there is no pdata
Hi Nick,
From: Yufeng Shen mile...@chromium.org
This is the preparation for supporting the code path when there is
platform data provided and still boot the device into a sane state
with backup NVRAM config.
Make the irqflags default to be IRQF_TRIGGER_FALLING if no platform data is
provided.
Signed-off-by: Yufeng Shen mile...@chromium.org
Signed-off-by: Daniel Kurtz djku...@chromium.org
Signed-off-by: Nick Dyer nick.d...@itdev.co.uk
Acked-by: Benson Leung ble...@chromium.org
---
drivers/input/touchscreen/atmel_mxt_ts.c | 54
+-
1 file changed, 39 insertions(+), 15 deletions(-)
diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c
b/drivers/input/touchscreen/atmel_mxt_ts.c
index 1334e5b..2645d36 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -232,7 +232,7 @@ struct mxt_data {
struct i2c_client *client;
struct input_dev *input_dev;
char phys[64]; /* device physical location */
- const struct mxt_platform_data *pdata;
+ struct mxt_platform_data *pdata;
struct mxt_object *object_table;
struct mxt_info info;
unsigned int irq;
@@ -1640,10 +1640,29 @@ static void mxt_input_close(struct input_dev *dev)
mxt_stop(data);
}
+static int mxt_handle_pdata(struct mxt_data *data)
+{
+ data-pdata = dev_get_platdata(data-client-dev);
+
+ /* Use provided platform data if present */
+ if (data-pdata)
+ return 0;
+
+ data-pdata = kzalloc(sizeof(*data-pdata), GFP_KERNEL);
+ if (!data-pdata) {
+ dev_err(data-client-dev, Failed to allocate pdata\n);
+ return -ENOMEM;
+ }
Any chance this could be a static instead?
+
+ /* Set default parameters */
+ data-pdata-irqflags = IRQF_TRIGGER_FALLING;
+
+ return 0;
+}
+
Opencode instead?
static int mxt_probe(struct i2c_client *client,
const struct i2c_device_id *id)
{
- const struct mxt_platform_data *pdata = client-dev.platform_data;
This line keeps reappearing in various versions of this
function. Perhaps it should simply stay as is instead?
struct mxt_data *data;
struct input_dev *input_dev;
int error;
@@ -1651,9 +1670,6 @@ static int mxt_probe(struct i2c_client *client,
unsigned int mt_flags = 0;
int i;
- if (!pdata)
Why not just initialize the default here instead?
- return -EINVAL;
-
data = kzalloc(sizeof(struct mxt_data), GFP_KERNEL);
input_dev = input_allocate_device();
if (!data || !input_dev) {
@@ -1675,19 +1691,23 @@ static int mxt_probe(struct i2c_client *client,
data-client = client;
data-input_dev = input_dev;
- data-pdata = pdata;
data-irq = client-irq;
+ i2c_set_clientdata(client, data);
+
+ error = mxt_handle_pdata(data);
+ if (error)
+ goto err_free_mem;
then this would go away
init_completion(data-bl_completion);
init_completion(data-reset_completion);
init_completion(data-crc_completion);
- error = request_threaded_irq(client-irq, NULL, mxt_interrupt,
- pdata-irqflags | IRQF_ONESHOT,
+ error = request_threaded_irq(data-irq, NULL, mxt_interrupt,
+ data-pdata-irqflags | IRQF_ONESHOT,
client-name, data);
and this hunk
if (error) {
dev_err(client-dev, Failed to register interrupt\n);
- goto err_free_mem;
+ goto err_free_pdata;
}
disable_irq(client-irq);
@@ -1700,13 +1720,13 @@ static int mxt_probe(struct i2c_client *client,
__set_bit(EV_KEY, input_dev-evbit);
__set_bit(BTN_TOUCH, input_dev-keybit);
- if (pdata-t19_num_keys) {
+ if (data-pdata-t19_num_keys) {
and this hunk
__set_bit(INPUT_PROP_BUTTONPAD, input_dev-propbit);
- for (i = 0; i pdata-t19_num_keys; i++)
- if (pdata-t19_keymap[i] != KEY_RESERVED)
+ for (i = 0; i data-pdata-t19_num_keys; i++)
+ if (data-pdata-t19_keymap[i] != KEY_RESERVED)
input_set_capability(input_dev, EV_KEY,
- pdata-t19_keymap[i]);
+ data-pdata-t19_keymap[i]);
mt_flags |= INPUT_MT_POINTER;
@@ -1743,7 +1763,6 @@ static int mxt_probe(struct i2c_client *client,
0, 255, 0, 0);
input_set_drvdata(input_dev, data);
- i2c_set_clientdata(client, data);
error = input_register_device(input_dev);
if (error) {
@@ -1767,7 +1786,10 @@ err_unregister_device:
err_free_object:
kfree(data-object_table);
err_free_irq:
- free_irq(client-irq, data);
+ free_irq(data-irq,
[PATCH 20/51] Input: atmel_mxt_ts - Set default irqflags when there is no pdata
From: Yufeng Shen
This is the preparation for supporting the code path when there is
platform data provided and still boot the device into a sane state
with backup NVRAM config.
Make the irqflags default to be IRQF_TRIGGER_FALLING if no platform data is
provided.
Signed-off-by: Yufeng Shen
Signed-off-by: Daniel Kurtz
Signed-off-by: Nick Dyer
Acked-by: Benson Leung
---
drivers/input/touchscreen/atmel_mxt_ts.c | 54 +-
1 file changed, 39 insertions(+), 15 deletions(-)
diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c
b/drivers/input/touchscreen/atmel_mxt_ts.c
index 1334e5b..2645d36 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -232,7 +232,7 @@ struct mxt_data {
struct i2c_client *client;
struct input_dev *input_dev;
char phys[64]; /* device physical location */
- const struct mxt_platform_data *pdata;
+ struct mxt_platform_data *pdata;
struct mxt_object *object_table;
struct mxt_info info;
unsigned int irq;
@@ -1640,10 +1640,29 @@ static void mxt_input_close(struct input_dev *dev)
mxt_stop(data);
}
+static int mxt_handle_pdata(struct mxt_data *data)
+{
+ data->pdata = dev_get_platdata(>client->dev);
+
+ /* Use provided platform data if present */
+ if (data->pdata)
+ return 0;
+
+ data->pdata = kzalloc(sizeof(*data->pdata), GFP_KERNEL);
+ if (!data->pdata) {
+ dev_err(>client->dev, "Failed to allocate pdata\n");
+ return -ENOMEM;
+ }
+
+ /* Set default parameters */
+ data->pdata->irqflags = IRQF_TRIGGER_FALLING;
+
+ return 0;
+}
+
static int mxt_probe(struct i2c_client *client,
const struct i2c_device_id *id)
{
- const struct mxt_platform_data *pdata = client->dev.platform_data;
struct mxt_data *data;
struct input_dev *input_dev;
int error;
@@ -1651,9 +1670,6 @@ static int mxt_probe(struct i2c_client *client,
unsigned int mt_flags = 0;
int i;
- if (!pdata)
- return -EINVAL;
-
data = kzalloc(sizeof(struct mxt_data), GFP_KERNEL);
input_dev = input_allocate_device();
if (!data || !input_dev) {
@@ -1675,19 +1691,23 @@ static int mxt_probe(struct i2c_client *client,
data->client = client;
data->input_dev = input_dev;
- data->pdata = pdata;
data->irq = client->irq;
+ i2c_set_clientdata(client, data);
+
+ error = mxt_handle_pdata(data);
+ if (error)
+ goto err_free_mem;
init_completion(>bl_completion);
init_completion(>reset_completion);
init_completion(>crc_completion);
- error = request_threaded_irq(client->irq, NULL, mxt_interrupt,
-pdata->irqflags | IRQF_ONESHOT,
+ error = request_threaded_irq(data->irq, NULL, mxt_interrupt,
+data->pdata->irqflags | IRQF_ONESHOT,
client->name, data);
if (error) {
dev_err(>dev, "Failed to register interrupt\n");
- goto err_free_mem;
+ goto err_free_pdata;
}
disable_irq(client->irq);
@@ -1700,13 +1720,13 @@ static int mxt_probe(struct i2c_client *client,
__set_bit(EV_KEY, input_dev->evbit);
__set_bit(BTN_TOUCH, input_dev->keybit);
- if (pdata->t19_num_keys) {
+ if (data->pdata->t19_num_keys) {
__set_bit(INPUT_PROP_BUTTONPAD, input_dev->propbit);
- for (i = 0; i < pdata->t19_num_keys; i++)
- if (pdata->t19_keymap[i] != KEY_RESERVED)
+ for (i = 0; i < data->pdata->t19_num_keys; i++)
+ if (data->pdata->t19_keymap[i] != KEY_RESERVED)
input_set_capability(input_dev, EV_KEY,
-pdata->t19_keymap[i]);
+ data->pdata->t19_keymap[i]);
mt_flags |= INPUT_MT_POINTER;
@@ -1743,7 +1763,6 @@ static int mxt_probe(struct i2c_client *client,
0, 255, 0, 0);
input_set_drvdata(input_dev, data);
- i2c_set_clientdata(client, data);
error = input_register_device(input_dev);
if (error) {
@@ -1767,7 +1786,10 @@ err_unregister_device:
err_free_object:
kfree(data->object_table);
err_free_irq:
- free_irq(client->irq, data);
+ free_irq(data->irq, data);
+err_free_pdata:
+ if (!dev_get_platdata(>client->dev))
+ kfree(data->pdata);
err_free_mem:
input_free_device(input_dev);
kfree(data);
@@ -1782,6 +1804,8 @@ static int mxt_remove(struct i2c_client *client)
free_irq(data->irq, data);
input_unregister_device(data->input_dev);
[PATCH 20/51] Input: atmel_mxt_ts - Set default irqflags when there is no pdata
From: Yufeng Shen mile...@chromium.org
This is the preparation for supporting the code path when there is
platform data provided and still boot the device into a sane state
with backup NVRAM config.
Make the irqflags default to be IRQF_TRIGGER_FALLING if no platform data is
provided.
Signed-off-by: Yufeng Shen mile...@chromium.org
Signed-off-by: Daniel Kurtz djku...@chromium.org
Signed-off-by: Nick Dyer nick.d...@itdev.co.uk
Acked-by: Benson Leung ble...@chromium.org
---
drivers/input/touchscreen/atmel_mxt_ts.c | 54 +-
1 file changed, 39 insertions(+), 15 deletions(-)
diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c
b/drivers/input/touchscreen/atmel_mxt_ts.c
index 1334e5b..2645d36 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -232,7 +232,7 @@ struct mxt_data {
struct i2c_client *client;
struct input_dev *input_dev;
char phys[64]; /* device physical location */
- const struct mxt_platform_data *pdata;
+ struct mxt_platform_data *pdata;
struct mxt_object *object_table;
struct mxt_info info;
unsigned int irq;
@@ -1640,10 +1640,29 @@ static void mxt_input_close(struct input_dev *dev)
mxt_stop(data);
}
+static int mxt_handle_pdata(struct mxt_data *data)
+{
+ data-pdata = dev_get_platdata(data-client-dev);
+
+ /* Use provided platform data if present */
+ if (data-pdata)
+ return 0;
+
+ data-pdata = kzalloc(sizeof(*data-pdata), GFP_KERNEL);
+ if (!data-pdata) {
+ dev_err(data-client-dev, Failed to allocate pdata\n);
+ return -ENOMEM;
+ }
+
+ /* Set default parameters */
+ data-pdata-irqflags = IRQF_TRIGGER_FALLING;
+
+ return 0;
+}
+
static int mxt_probe(struct i2c_client *client,
const struct i2c_device_id *id)
{
- const struct mxt_platform_data *pdata = client-dev.platform_data;
struct mxt_data *data;
struct input_dev *input_dev;
int error;
@@ -1651,9 +1670,6 @@ static int mxt_probe(struct i2c_client *client,
unsigned int mt_flags = 0;
int i;
- if (!pdata)
- return -EINVAL;
-
data = kzalloc(sizeof(struct mxt_data), GFP_KERNEL);
input_dev = input_allocate_device();
if (!data || !input_dev) {
@@ -1675,19 +1691,23 @@ static int mxt_probe(struct i2c_client *client,
data-client = client;
data-input_dev = input_dev;
- data-pdata = pdata;
data-irq = client-irq;
+ i2c_set_clientdata(client, data);
+
+ error = mxt_handle_pdata(data);
+ if (error)
+ goto err_free_mem;
init_completion(data-bl_completion);
init_completion(data-reset_completion);
init_completion(data-crc_completion);
- error = request_threaded_irq(client-irq, NULL, mxt_interrupt,
-pdata-irqflags | IRQF_ONESHOT,
+ error = request_threaded_irq(data-irq, NULL, mxt_interrupt,
+data-pdata-irqflags | IRQF_ONESHOT,
client-name, data);
if (error) {
dev_err(client-dev, Failed to register interrupt\n);
- goto err_free_mem;
+ goto err_free_pdata;
}
disable_irq(client-irq);
@@ -1700,13 +1720,13 @@ static int mxt_probe(struct i2c_client *client,
__set_bit(EV_KEY, input_dev-evbit);
__set_bit(BTN_TOUCH, input_dev-keybit);
- if (pdata-t19_num_keys) {
+ if (data-pdata-t19_num_keys) {
__set_bit(INPUT_PROP_BUTTONPAD, input_dev-propbit);
- for (i = 0; i pdata-t19_num_keys; i++)
- if (pdata-t19_keymap[i] != KEY_RESERVED)
+ for (i = 0; i data-pdata-t19_num_keys; i++)
+ if (data-pdata-t19_keymap[i] != KEY_RESERVED)
input_set_capability(input_dev, EV_KEY,
-pdata-t19_keymap[i]);
+ data-pdata-t19_keymap[i]);
mt_flags |= INPUT_MT_POINTER;
@@ -1743,7 +1763,6 @@ static int mxt_probe(struct i2c_client *client,
0, 255, 0, 0);
input_set_drvdata(input_dev, data);
- i2c_set_clientdata(client, data);
error = input_register_device(input_dev);
if (error) {
@@ -1767,7 +1786,10 @@ err_unregister_device:
err_free_object:
kfree(data-object_table);
err_free_irq:
- free_irq(client-irq, data);
+ free_irq(data-irq, data);
+err_free_pdata:
+ if (!dev_get_platdata(data-client-dev))
+ kfree(data-pdata);
err_free_mem:
input_free_device(input_dev);
kfree(data);
@@ -1782,6 +1804,8 @@ static int mxt_remove(struct i2c_client *client)
free_irq(data-irq,
