Re: [PATCH 1/4] dt-bindings: leds: document property for LED triggers

[Jacek Anaszewski <jacek.anaszewski-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]

On 02/28/2017 10:51 PM, Rafał Miłecki wrote:
> On 02/28/2017 10:38 PM, Jacek Anaszewski wrote:
>> I think that it would be simpler if we could initially see
>> a complete sample dts implementation containing all required DT
>> nodes. The example could contain timer trigger as well as usb-port
>> trigger specific bindings.
> 
> Please take a look at attached patch. I used it on Tenda AC9 with:
> 
> usb_trigger: usb-trigger {
>     trigger-type = "usbport";
>     ports = <&ohci_port1>, <&ehci_port1>;
> };
> 
> usb {
>     label = "bcm53xx:blue:usb";
>     gpios = <&chipcommon 1 GPIO_ACTIVE_HIGH>;
>     triggers = <&usb_trigger>;
> };

OK, I got it, thanks.

> 
>> I suppose that we should see DT nodes containing #list-cells
>> properties that define the quantity of phandle arguments.
>>
>> It seems that this approach allows for defining a list of elements
>> with variable number of arguments, i.e. what you were initially
>> asking for.
> 
> Are you sure we need #list-cells? Can't we simply use
> of_count_phandle_with_args(np, "triggers", NULL);
> ?

I'm not sure, I just read the function documentation :-)
I haven't verified nor have I used this API.

> 
> From: =?UTF-8?q?Rafa=C5=82=20Mi=C5=82ecki?= <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
> Date: Mon, 3 Oct 2016 16:49:05 +0200
> Subject: [PATCH] usb: core: read USB ports from DT in the usbport LED
> trigger
>  driver
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
> 
> Signed-off-by: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
> ---
>  .../devicetree/bindings/leds/triggers-usbport.txt  | 19 ++++++++
>  drivers/leds/led-triggers.c                        |  3 +-
>  drivers/usb/core/ledtrig-usbport.c                 | 56
> ++++++++++++++++++++++
>  3 files changed, 77 insertions(+), 1 deletion(-)
>  create mode 100644
> Documentation/devicetree/bindings/leds/triggers-usbport.txt
> 
> diff --git a/Documentation/devicetree/bindings/leds/triggers-usbport.txt
> b/Documentation/devicetree/bindings/leds/triggers-usbport.txt
> new file mode 100644
> index 000000000000..10e55122cded
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/leds/triggers-usbport.txt
> @@ -0,0 +1,19 @@
> +USB port LED trigger properties.
> +
> +USB port trigger can be used for signalling to the user a presence of USB
> +device(s) in given ports. It's possible to specify USB ports in DT by
> providing
> +their references.
> +
> +Properties:
> +- trigger-type : Must be "usbport".
> +- ports : List of USB ports related to this LED. Some devices may have
> one USB
> +      LED for all ports, other may have few of them (e.g. USB version
> +      specific). It's used by usbport trigger for reading a list of ports
> +      that should cause LED to turn on whenver device get connected.
> +
> +Examples:
> +
> +usbport-trigger {
> +    trigger-type = "usbport";
> +    ports = <&ohci_port1>, <&ehci_port1>;
> +};
> diff --git a/drivers/leds/led-triggers.c b/drivers/leds/led-triggers.c
> index c53c20d676cd..1d8bda9755ca 100644
> --- a/drivers/leds/led-triggers.c
> +++ b/drivers/leds/led-triggers.c
> @@ -181,7 +181,8 @@ static void led_trigger_of_read_trigger(struct
> led_classdev *led_cdev)
>      }
> 
>      /* Check if trigger specified in DT is supported */
> -    if (strcmp(trigger_type, "timer"))
> +    if (strcmp(trigger_type, "timer") &&
> +        strcmp(trigger_type, "usbport"))
>          goto err_node_put;
> 
>      led_cdev->default_trigger = trigger_type;
> diff --git a/drivers/usb/core/ledtrig-usbport.c
> b/drivers/usb/core/ledtrig-usbport.c
> index 1713248ab15a..599f7e6b0ba8 100644
> --- a/drivers/usb/core/ledtrig-usbport.c
> +++ b/drivers/usb/core/ledtrig-usbport.c
> @@ -11,8 +11,10 @@
>  #include <linux/device.h>
>  #include <linux/leds.h>
>  #include <linux/module.h>
> +#include <linux/of.h>
>  #include <linux/slab.h>
>  #include <linux/usb.h>
> +#include <linux/usb/of.h>
> 
>  struct usbport_trig_data {
>      struct led_classdev *led_cdev;
> @@ -123,6 +125,58 @@ static const struct attribute_group ports_group = {
>   * Adding & removing ports
>   ***************************************/
> 
> +/**
> + * usbport_trig_port_observed - Check if port should be observed
> + *
> + * Each LED may have list of related USB ports specified in a DT. This
> function
> + * reads them using ports property and sets a proper state.
> + */
> +static bool usbport_trig_port_observed(struct usbport_trig_data
> *usbport_data,
> +                       struct usb_device *usb_dev, int port1)
> +{
> +    struct device_node *led_np = usbport_data->led_cdev->trigger_node;
> +    struct device *dev = usbport_data->led_cdev->dev;
> +    struct of_phandle_args args;
> +    struct device_node *port_np;
> +    int count, i;
> +
> +    if (!led_np)
> +        return false;
> +
> +    /* Get node of port being added */
> +    port_np = usb_of_get_child_node(usb_dev->dev.of_node, port1);
> +    if (!port_np)
> +        return false;
> +
> +    /* Amount of ports this LED references */
> +    count = of_count_phandle_with_args(led_np, "ports", NULL);
> +    if (count < 0) {
> +        dev_warn(dev, "Failed to get USB ports for %s\n",
> +             led_np->full_name);
> +        return false;
> +    }
> +
> +    /* Check if port is on this LED's list */
> +    for (i = 0; i < count; i++) {
> +        int err;
> +
> +        err = of_parse_phandle_with_args(led_np, "ports", NULL, i,
> +                         &args);
> +        if (err) {
> +            dev_err(dev, "Failed to get USB port phandle at index %d:
> %d\n",
> +                i, err);
> +            continue;
> +        }
> +
> +        of_node_put(args.np);
> +
> +        if (args.np == port_np)
> +            return true;
> +    }
> +
> +    return false;
> +}
> +
>  static int usbport_trig_add_port(struct usbport_trig_data *usbport_data,
>                   struct usb_device *usb_dev,
>                   const char *hub_name, int portnum)
> @@ -141,6 +195,7 @@ static int usbport_trig_add_port(struct
> usbport_trig_data *usbport_data,
>      port->data = usbport_data;
>      port->hub = usb_dev;
>      port->portnum = portnum;
> +    port->observed = usbport_trig_port_observed(usbport_data, usb_dev,
> portnum);
> 
>      len = strlen(hub_name) + 8;
>      port->port_name = kzalloc(len, GFP_KERNEL);
> @@ -255,6 +310,7 @@ static void usbport_trig_activate(struct
> led_classdev *led_cdev)
>      if (err)
>          goto err_free;
>      usb_for_each_dev(usbport_data, usbport_trig_add_usb_dev_ports);
> +    usbport_trig_update_count(usbport_data);
> 
>      /* Notifications */
>      usbport_data->nb.notifier_call = usbport_trig_notify,

-- 
Best regards,
Jacek Anaszewski
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html