Re: [PATCH 4/9] iio: adc: at91_adc: rework trigger definition

From: Jonathan Cameron
Date: Sat Nov 14 2020 - 12:02:57 EST

Next message: Stephen Boyd: "Re: [PATCH] clk: qcom: lpass-sc7180: Clean up on error in lpass_sc7180_init()"
Previous message: Jonathan Cameron: "Re: [PATCH 2/9] iio: adc: at91_adc: rework resolution selection"
In reply to: Alexandre Belloni: "[PATCH 4/9] iio: adc: at91_adc: rework trigger definition"
Next in thread: Alexandre Belloni: "Re: [PATCH 4/9] iio: adc: at91_adc: rework trigger definition"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On Fri, 13 Nov 2020 22:26:45 +0100
Alexandre Belloni <alexandre.belloni@xxxxxxxxxxx> wrote:

> Move the available trigger definition back in the driver to stop cluttering
> the device tree. There is no functional change except that it actually
> fixes the available triggers for at91sam9rl as it inherited the list from
> at91sam9260 but actually has the triggers from at91sam9x5.

Is that a fix we might want to backport? If not we should probably put a clear
statement in here to avoid it getting picked up by the bot.

I'd argue it's to invasive a change to backport.

Otherwise, sensible looking change.

>
> Signed-off-by: Alexandre Belloni <alexandre.belloni@xxxxxxxxxxx>
> ---
> .../bindings/iio/adc/atmel,sama9260-adc.yaml | 46 -----------
> drivers/iio/adc/at91_adc.c | 80 +++++++++----------
> 2 files changed, 36 insertions(+), 90 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/iio/adc/atmel,sama9260-adc.yaml b/Documentation/devicetree/bindings/iio/adc/atmel,sama9260-adc.yaml
> index 9b0ff59e75de..e6a1f915b542 100644
> --- a/Documentation/devicetree/bindings/iio/adc/atmel,sama9260-adc.yaml
> +++ b/Documentation/devicetree/bindings/iio/adc/atmel,sama9260-adc.yaml
> @@ -97,29 +97,6 @@ required:
> - atmel,adc-startup-time
> - atmel,adc-vref
>
> -patternProperties:
> - "^(trigger)[0-9]$":
> - type: object
> - description: Child node to describe a trigger exposed to the user.
> - properties:
> - trigger-name:
> - $ref: /schemas/types.yaml#/definitions/string
> - description: Identifying name.
> -
> - trigger-value:
> - $ref: /schemas/types.yaml#/definitions/uint32
> - description:
> - Value to put in the Trigger register to activate this trigger
> -
> - trigger-external:
> - $ref: /schemas/types.yaml#/definitions/flag
> - description: This trigger is provided from an external pin.
> -
> - additionalProperties: false
> - required:
> - - trigger-name
> - - trigger-value
> -
> examples:
> - |
> #include <dt-bindings/dma/at91.h>
> @@ -139,29 +116,6 @@ examples:
> atmel,adc-use-external-triggers;
> atmel,adc-vref = <3300>;
> atmel,adc-use-res = "lowres";
> -
> - trigger0 {
> - trigger-name = "external-rising";
> - trigger-value = <0x1>;
> - trigger-external;
> - };
> -
> - trigger1 {
> - trigger-name = "external-falling";
> - trigger-value = <0x2>;
> - trigger-external;
> - };
> -
> - trigger2 {
> - trigger-name = "external-any";
> - trigger-value = <0x3>;
> - trigger-external;
> - };
> -
> - trigger3 {
> - trigger-name = "continuous";
> - trigger-value = <0x6>;
> - };
> };
> };
> ...
> diff --git a/drivers/iio/adc/at91_adc.c b/drivers/iio/adc/at91_adc.c
> index 0d67c812ef3d..83539422b704 100644
> --- a/drivers/iio/adc/at91_adc.c
> +++ b/drivers/iio/adc/at91_adc.c
> @@ -207,6 +207,8 @@ struct at91_adc_caps {
>
> u8 low_res_bits;
> u8 high_res_bits;
> + u32 trigger_number;
> + const struct at91_adc_trigger *triggers;
> struct at91_adc_reg_desc registers;
> };
>
> @@ -227,8 +229,6 @@ struct at91_adc_state {
> u8 sample_hold_time;
> bool sleep_mode;
> struct iio_trigger **trig;
> - struct at91_adc_trigger *trigger_list;
> - u32 trigger_number;
> bool use_external;
> u32 vref_mv;
> u32 res; /* resolution used for convertions */
> @@ -537,13 +537,13 @@ static int at91_adc_channel_init(struct iio_dev *idev)
> }
>
> static int at91_adc_get_trigger_value_by_name(struct iio_dev *idev,
> - struct at91_adc_trigger *triggers,
> + const struct at91_adc_trigger *triggers,
> const char *trigger_name)
> {
> struct at91_adc_state *st = iio_priv(idev);
> int i;
>
> - for (i = 0; i < st->trigger_number; i++) {
> + for (i = 0; i < st->caps->trigger_number; i++) {
> char *name = kasprintf(GFP_KERNEL,
> "%s-dev%d-%s",
> idev->name,
> @@ -575,7 +575,7 @@ static int at91_adc_configure_trigger(struct iio_trigger *trig, bool state)
> u8 bit;
>
> value = at91_adc_get_trigger_value_by_name(idev,
> - st->trigger_list,
> + st->caps->triggers,
> idev->trig->name);
> if (value < 0)
> return value;
> @@ -620,7 +620,7 @@ static const struct iio_trigger_ops at91_adc_trigger_ops = {
> };
>
> static struct iio_trigger *at91_adc_allocate_trigger(struct iio_dev *idev,
> - struct at91_adc_trigger *trigger)
> + const struct at91_adc_trigger *trigger)
> {
> struct iio_trigger *trig;
> int ret;
> @@ -647,7 +647,7 @@ static int at91_adc_trigger_init(struct iio_dev *idev)
> int i, ret;
>
> st->trig = devm_kcalloc(&idev->dev,
> - st->trigger_number, sizeof(*st->trig),
> + st->caps->trigger_number, sizeof(*st->trig),
> GFP_KERNEL);
>
> if (st->trig == NULL) {
> @@ -655,12 +655,12 @@ static int at91_adc_trigger_init(struct iio_dev *idev)
> goto error_ret;
> }
>
> - for (i = 0; i < st->trigger_number; i++) {
> - if (st->trigger_list[i].is_external && !(st->use_external))
> + for (i = 0; i < st->caps->trigger_number; i++) {
> + if (st->caps->triggers[i].is_external && !(st->use_external))
> continue;
>
> st->trig[i] = at91_adc_allocate_trigger(idev,
> - st->trigger_list + i);
> + st->caps->triggers + i);
> if (st->trig[i] == NULL) {
> dev_err(&idev->dev,
> "Could not allocate trigger %d\n", i);
> @@ -685,7 +685,7 @@ static void at91_adc_trigger_remove(struct iio_dev *idev)
> struct at91_adc_state *st = iio_priv(idev);
> int i;
>
> - for (i = 0; i < st->trigger_number; i++) {
> + for (i = 0; i < st->caps->trigger_number; i++) {
> iio_trigger_unregister(st->trig[i]);
> iio_trigger_free(st->trig[i]);
> }
> @@ -838,8 +838,7 @@ static int at91_adc_probe_dt(struct iio_dev *idev,
> {
> struct at91_adc_state *st = iio_priv(idev);
> struct device_node *node = pdev->dev.of_node;
> - struct device_node *trig_node;
> - int i = 0, ret;
> + int ret;
> u32 prop;
> char *s;
>
> @@ -885,37 +884,6 @@ static int at91_adc_probe_dt(struct iio_dev *idev,
>
> st->registers = &st->caps->registers;
> st->num_channels = st->caps->num_channels;
> - st->trigger_number = of_get_child_count(node);
> - st->trigger_list = devm_kcalloc(&idev->dev,
> - st->trigger_number,
> - sizeof(struct at91_adc_trigger),
> - GFP_KERNEL);
> - if (!st->trigger_list) {
> - dev_err(&idev->dev, "Could not allocate trigger list memory.\n");
> - ret = -ENOMEM;
> - goto error_ret;
> - }
> -
> - for_each_child_of_node(node, trig_node) {
> - struct at91_adc_trigger *trig = st->trigger_list + i;
> - const char *name;
> -
> - if (of_property_read_string(trig_node, "trigger-name", &name)) {
> - dev_err(&idev->dev, "Missing trigger-name property in the DT.\n");
> - ret = -EINVAL;
> - goto error_ret;
> - }
> - trig->name = name;
> -
> - if (of_property_read_u32(trig_node, "trigger-value", &prop)) {
> - dev_err(&idev->dev, "Missing trigger-value property in the DT.\n");
> - ret = -EINVAL;
> - goto error_ret;
> - }
> - trig->value = prop;
> - trig->is_external = of_property_read_bool(trig_node, "trigger-external");
> - i++;
> - }
>
> /* Check if touchscreen is supported. */
> if (st->caps->has_ts)
> @@ -1315,6 +1283,13 @@ static int at91_adc_resume(struct device *dev)
>
> static SIMPLE_DEV_PM_OPS(at91_adc_pm_ops, at91_adc_suspend, at91_adc_resume);
>
> +static const struct at91_adc_trigger at91sam9260_triggers[] = {
> + { .name = "timer-counter-0", .value = 0x1 },
> + { .name = "timer-counter-1", .value = 0x3 },
> + { .name = "timer-counter-2", .value = 0x5 },
> + { .name = "external", .value = 0xd, .is_external = true },
> +};
> +
> static struct at91_adc_caps at91sam9260_caps = {
> .calc_startup_ticks = calc_startup_ticks_9260,
> .num_channels = 4,
> @@ -1328,6 +1303,15 @@ static struct at91_adc_caps at91sam9260_caps = {
> .mr_prescal_mask = AT91_ADC_PRESCAL_9260,
> .mr_startup_mask = AT91_ADC_STARTUP_9260,
> },
> + .triggers = at91sam9260_triggers,
> + .trigger_number = ARRAY_SIZE(at91sam9260_triggers),
> +};
> +
> +static const struct at91_adc_trigger at91sam9x5_triggers[] = {
> + { .name = "external-rising", .value = 0x1, .is_external = true },
> + { .name = "external-falling", .value = 0x2, .is_external = true },
> + { .name = "external-any", .value = 0x3, .is_external = true },
> + { .name = "continuous", .value = 0x6 },
> };
>
> static struct at91_adc_caps at91sam9rl_caps = {
> @@ -1344,6 +1328,8 @@ static struct at91_adc_caps at91sam9rl_caps = {
> .mr_prescal_mask = AT91_ADC_PRESCAL_9260,
> .mr_startup_mask = AT91_ADC_STARTUP_9G45,
> },
> + .triggers = at91sam9x5_triggers,
> + .trigger_number = ARRAY_SIZE(at91sam9x5_triggers),
> };
>
> static struct at91_adc_caps at91sam9g45_caps = {
> @@ -1360,6 +1346,8 @@ static struct at91_adc_caps at91sam9g45_caps = {
> .mr_prescal_mask = AT91_ADC_PRESCAL_9G45,
> .mr_startup_mask = AT91_ADC_STARTUP_9G45,
> },
> + .triggers = at91sam9x5_triggers,
> + .trigger_number = ARRAY_SIZE(at91sam9x5_triggers),
> };
>
> static struct at91_adc_caps at91sam9x5_caps = {
> @@ -1380,6 +1368,8 @@ static struct at91_adc_caps at91sam9x5_caps = {
> .mr_prescal_mask = AT91_ADC_PRESCAL_9G45,
> .mr_startup_mask = AT91_ADC_STARTUP_9X5,
> },
> + .triggers = at91sam9x5_triggers,
> + .trigger_number = ARRAY_SIZE(at91sam9x5_triggers),
> };
>
> static struct at91_adc_caps sama5d3_caps = {
> @@ -1399,6 +1389,8 @@ static struct at91_adc_caps sama5d3_caps = {
> .mr_prescal_mask = AT91_ADC_PRESCAL_9G45,
> .mr_startup_mask = AT91_ADC_STARTUP_9X5,
> },
> + .triggers = at91sam9x5_triggers,
> + .trigger_number = ARRAY_SIZE(at91sam9x5_triggers),
> };
>
> static const struct of_device_id at91_adc_dt_ids[] = {

Next message: Stephen Boyd: "Re: [PATCH] clk: qcom: lpass-sc7180: Clean up on error in lpass_sc7180_init()"
Previous message: Jonathan Cameron: "Re: [PATCH 2/9] iio: adc: at91_adc: rework resolution selection"
In reply to: Alexandre Belloni: "[PATCH 4/9] iio: adc: at91_adc: rework trigger definition"
Next in thread: Alexandre Belloni: "Re: [PATCH 4/9] iio: adc: at91_adc: rework trigger definition"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]