| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <871ru0x8ws.fsf@FE-laptop>
Date: Fri, 22 Nov 2019 15:50:11 +0100
From: Gregory CLEMENT <gregory.clement@...tlin.com>
To: Cristian Birsan <cristian.birsan@...rochip.com>,
Felipe Balbi <balbi@...nel.org>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Alan Stern <stern@...land.harvard.edu>,
linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org
Cc: Rob Herring <robh+dt@...nel.org>, devicetree@...r.kernel.org,
Nicolas Ferre <nicolas.ferre@...rochip.com>,
Alexandre Belloni <alexandre.belloni@...tlin.com>,
Ludovic Desroches <ludovic.desroches@...rochip.com>,
linux-arm-kernel@...ts.infradead.org,
Thomas Petazzoni <thomas.petazzoni@...tlin.com>
Subject: Re: [PATCH 1/3] usb: gadget: udc: atmel: Don't use DT to configure end point
Hello,
> The endpoint configuration used to be stored in the device tree,
> however the configuration depend on the "version" of the controller
> itself.
>
> This information is already documented by the compatible string. It
> then possible to just rely on the compatible string and completely
> remove the full ep configuration done in the device tree as it was
> already the case for all the other USB device controller.
Do you have any feedback about this patch ?
The binding being reviewed is there any chance that this patch will be
merged?
Thanks,
Gregory
>
> Signed-off-by: Gregory CLEMENT <gregory.clement@...tlin.com>
> ---
> drivers/usb/gadget/udc/atmel_usba_udc.c | 112 +++++++++++++++---------
> drivers/usb/gadget/udc/atmel_usba_udc.h | 12 +++
> 2 files changed, 84 insertions(+), 40 deletions(-)
>
> diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.c b/drivers/usb/gadget/udc/atmel_usba_udc.c
> index 86ffc8307864..2db833caeb09 100644
> --- a/drivers/usb/gadget/udc/atmel_usba_udc.c
> +++ b/drivers/usb/gadget/udc/atmel_usba_udc.c
> @@ -2040,10 +2040,56 @@ static const struct usba_udc_errata at91sam9g45_errata = {
> .pulse_bias = at91sam9g45_pulse_bias,
> };
>
> +static const struct usba_ep_config ep_config_sam9[] __initconst = {
> + { .nr_banks = 1 }, /* ep 0 */
> + { .nr_banks = 2, .can_dma = 1, .can_isoc = 1 }, /* ep 1 */
> + { .nr_banks = 2, .can_dma = 1, .can_isoc = 1 }, /* ep 2 */
> + { .nr_banks = 3, .can_dma = 1 }, /* ep 3 */
> + { .nr_banks = 3, .can_dma = 1 }, /* ep 4 */
> + { .nr_banks = 3, .can_dma = 1, .can_isoc = 1 }, /* ep 5 */
> + { .nr_banks = 3, .can_dma = 1, .can_isoc = 1 }, /* ep 6 */
> +};
> +
> +static const struct usba_ep_config ep_config_sama5[] __initconst = {
> + { .nr_banks = 1 }, /* ep 0 */
> + { .nr_banks = 3, .can_dma = 1, .can_isoc = 1 }, /* ep 1 */
> + { .nr_banks = 3, .can_dma = 1, .can_isoc = 1 }, /* ep 2 */
> + { .nr_banks = 2, .can_dma = 1, .can_isoc = 1 }, /* ep 3 */
> + { .nr_banks = 2, .can_dma = 1, .can_isoc = 1 }, /* ep 4 */
> + { .nr_banks = 2, .can_dma = 1, .can_isoc = 1 }, /* ep 5 */
> + { .nr_banks = 2, .can_dma = 1, .can_isoc = 1 }, /* ep 6 */
> + { .nr_banks = 2, .can_dma = 1, .can_isoc = 1 }, /* ep 7 */
> + { .nr_banks = 2, .can_isoc = 1 }, /* ep 8 */
> + { .nr_banks = 2, .can_isoc = 1 }, /* ep 9 */
> + { .nr_banks = 2, .can_isoc = 1 }, /* ep 10 */
> + { .nr_banks = 2, .can_isoc = 1 }, /* ep 11 */
> + { .nr_banks = 2, .can_isoc = 1 }, /* ep 12 */
> + { .nr_banks = 2, .can_isoc = 1 }, /* ep 13 */
> + { .nr_banks = 2, .can_isoc = 1 }, /* ep 14 */
> + { .nr_banks = 2, .can_isoc = 1 }, /* ep 15 */
> +};
> +
> +static const struct usba_udc_config udc_at91sam9rl_cfg = {
> + .errata = &at91sam9rl_errata,
> + .config = ep_config_sam9,
> + .num_ep = ARRAY_SIZE(ep_config_sam9),
> +};
> +
> +static const struct usba_udc_config udc_at91sam9g45_cfg = {
> + .errata = &at91sam9g45_errata,
> + .config = ep_config_sam9,
> + .num_ep = ARRAY_SIZE(ep_config_sam9),
> +};
> +
> +static const struct usba_udc_config udc_sama5d3_cfg = {
> + .config = ep_config_sama5,
> + .num_ep = ARRAY_SIZE(ep_config_sama5),
> +};
> +
> static const struct of_device_id atmel_udc_dt_ids[] = {
> - { .compatible = "atmel,at91sam9rl-udc", .data = &at91sam9rl_errata },
> - { .compatible = "atmel,at91sam9g45-udc", .data = &at91sam9g45_errata },
> - { .compatible = "atmel,sama5d3-udc" },
> + { .compatible = "atmel,at91sam9rl-udc", .data = &udc_at91sam9rl_cfg },
> + { .compatible = "atmel,at91sam9g45-udc", .data = &udc_at91sam9g45_cfg },
> + { .compatible = "atmel,sama5d3-udc", .data = &udc_sama5d3_cfg },
> { /* sentinel */ }
> };
>
> @@ -2052,18 +2098,19 @@ MODULE_DEVICE_TABLE(of, atmel_udc_dt_ids);
> static struct usba_ep * atmel_udc_of_init(struct platform_device *pdev,
> struct usba_udc *udc)
> {
> - u32 val;
> struct device_node *np = pdev->dev.of_node;
> const struct of_device_id *match;
> struct device_node *pp;
> int i, ret;
> struct usba_ep *eps, *ep;
> + const struct usba_udc_config *udc_config;
>
> match = of_match_node(atmel_udc_dt_ids, np);
> if (!match)
> return ERR_PTR(-EINVAL);
>
> - udc->errata = match->data;
> + udc_config = match->data;
> + udc->errata = udc_config->errata;
> udc->pmc = syscon_regmap_lookup_by_compatible("atmel,at91sam9g45-pmc");
> if (IS_ERR(udc->pmc))
> udc->pmc = syscon_regmap_lookup_by_compatible("atmel,at91sam9rl-pmc");
> @@ -2079,8 +2126,7 @@ static struct usba_ep * atmel_udc_of_init(struct platform_device *pdev,
>
> if (fifo_mode == 0) {
> pp = NULL;
> - while ((pp = of_get_next_child(np, pp)))
> - udc->num_ep++;
> + udc->num_ep = udc_config->num_ep;
> udc->configured_ep = 1;
> } else {
> udc->num_ep = usba_config_fifo_table(udc);
> @@ -2097,52 +2143,38 @@ static struct usba_ep * atmel_udc_of_init(struct platform_device *pdev,
>
> pp = NULL;
> i = 0;
> - while ((pp = of_get_next_child(np, pp)) && i < udc->num_ep) {
> + while (i < udc->num_ep) {
> + const struct usba_ep_config *ep_cfg = &udc_config->config[i];
> +
> ep = &eps[i];
>
> - ret = of_property_read_u32(pp, "reg", &val);
> - if (ret) {
> - dev_err(&pdev->dev, "of_probe: reg error(%d)\n", ret);
> - goto err;
> - }
> - ep->index = fifo_mode ? udc->fifo_cfg[i].hw_ep_num : val;
> + ep->index = fifo_mode ? udc->fifo_cfg[i].hw_ep_num : i;
> +
> + /* Only the first EP is 64 bytes */
> + if (ep->index == 0)
> + ep->fifo_size = 64;
> + else
> + ep->fifo_size = 1024;
>
> - ret = of_property_read_u32(pp, "atmel,fifo-size", &val);
> - if (ret) {
> - dev_err(&pdev->dev, "of_probe: fifo-size error(%d)\n", ret);
> - goto err;
> - }
> if (fifo_mode) {
> - if (val < udc->fifo_cfg[i].fifo_size) {
> + if (ep->fifo_size < udc->fifo_cfg[i].fifo_size)
> dev_warn(&pdev->dev,
> - "Using max fifo-size value from DT\n");
> - ep->fifo_size = val;
> - } else {
> + "Using default max fifo-size value\n");
> + else
> ep->fifo_size = udc->fifo_cfg[i].fifo_size;
> - }
> - } else {
> - ep->fifo_size = val;
> }
>
> - ret = of_property_read_u32(pp, "atmel,nb-banks", &val);
> - if (ret) {
> - dev_err(&pdev->dev, "of_probe: nb-banks error(%d)\n", ret);
> - goto err;
> - }
> + ep->nr_banks = ep_cfg->nr_banks;
> if (fifo_mode) {
> - if (val < udc->fifo_cfg[i].nr_banks) {
> + if (ep->nr_banks < udc->fifo_cfg[i].nr_banks)
> dev_warn(&pdev->dev,
> - "Using max nb-banks value from DT\n");
> - ep->nr_banks = val;
> - } else {
> + "Using default max nb-banks value\n");
> + else
> ep->nr_banks = udc->fifo_cfg[i].nr_banks;
> - }
> - } else {
> - ep->nr_banks = val;
> }
>
> - ep->can_dma = of_property_read_bool(pp, "atmel,can-dma");
> - ep->can_isoc = of_property_read_bool(pp, "atmel,can-isoc");
> + ep->can_dma = ep_cfg->can_dma;
> + ep->can_isoc = ep_cfg->can_isoc;
>
> sprintf(ep->name, "ep%d", ep->index);
> ep->ep.name = ep->name;
> diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.h b/drivers/usb/gadget/udc/atmel_usba_udc.h
> index a0225e4543d4..48e332439ed5 100644
> --- a/drivers/usb/gadget/udc/atmel_usba_udc.h
> +++ b/drivers/usb/gadget/udc/atmel_usba_udc.h
> @@ -290,6 +290,12 @@ struct usba_ep {
> #endif
> };
>
> +struct usba_ep_config {
> + u8 nr_banks;
> + unsigned int can_dma:1;
> + unsigned int can_isoc:1;
> +};
> +
> struct usba_request {
> struct usb_request req;
> struct list_head queue;
> @@ -307,6 +313,12 @@ struct usba_udc_errata {
> void (*pulse_bias)(struct usba_udc *udc);
> };
>
> +struct usba_udc_config {
> + const struct usba_udc_errata *errata;
> + const struct usba_ep_config *config;
> + const int num_ep;
> +};
> +
> struct usba_udc {
> /* Protect hw registers from concurrent modifications */
> spinlock_t lock;
> --
> 2.24.0.rc1
>
--
Gregory Clement, Bootlin
Embedded Linux and Kernel engineering
http://bootlin.com
Powered by blists - more mailing lists