On 09/16/2017 12:42 PM, Serge Semin wrote:
> USB251xb as well as USB2517 datasheet states, that all these
> hubs differ by number of ports declared as the last digit in the
> model name. So USB2512 got two ports, USB2513 - three, and so on.
> Such setting must be reflected in the device specific data
> structure and corresponding dts property should be checked whether
> it doesn't get out of available ports.
>
> Signed-off-by: Serge Semin <fancer.lancer@xxxxxxxxx>
> ---
> drivers/usb/misc/usb251xb.c | 21 ++++++++++++++++++---
> 1 file changed, 18 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/usb/misc/usb251xb.c b/drivers/usb/misc/usb251xb.c
> index 96a8c20ac..5cb0e5570 100644
> --- a/drivers/usb/misc/usb251xb.c
> +++ b/drivers/usb/misc/usb251xb.c
...
>
> @@ -422,8 +431,10 @@ static int usb251xb_get_ofdata(struct usb251xb *hub,
> for (i = 0; i < len / sizeof(u32); i++) {
> u32 port = be32_to_cpu(cproperty_u32[i]);
>
> - if ((port >= 1) && (port <= 4))
> + if ((port >= 1) && (port <= data->port_cnt))
> hub->non_rem_dev |= BIT(port);
> + else
> + dev_warn(dev, "port %u doesn't exist\n", port);
I'd prefer to add the (invalid) property trying to be set in this
warning messages. So someone is able to find the invalid dt setting
faster. Something like;
dev_warn(dev, "requested NRD port %u doesn't exist\n", port);
> }
> }
>
> @@ -433,8 +444,10 @@ static int usb251xb_get_ofdata(struct usb251xb *hub,
> for (i = 0; i < len / sizeof(u32); i++) {
> u32 port = be32_to_cpu(cproperty_u32[i]);
>
> - if ((port >= 1) && (port <= 4))
> + if ((port >= 1) && (port <= data->port_cnt))
> hub->port_disable_sp |= BIT(port);
> + else
> + dev_warn(dev, "port %u doesn't exist\n", port);
... same as above. For example:
dev_warn(dev, "requested PDS port %u doesn't exist\n", port);
> }
> }
>
> @@ -444,8 +457,10 @@ static int usb251xb_get_ofdata(struct usb251xb *hub,
> for (i = 0; i < len / sizeof(u32); i++) {
> u32 port = be32_to_cpu(cproperty_u32[i]);
>
> - if ((port >= 1) && (port <= 4))
> + if ((port >= 1) && (port <= data->port_cnt))
> hub->port_disable_bp |= BIT(port);
> + else
> + dev_warn(dev, "port %u doesn't exist\n", port);
... same as above. For example:
dev_warn(dev, "requested PDB port %u doesn't exist\n", port);
Apart from that this patch looks fine for me. Thanks for the spot of the
hardcoded max ports check.
regards,
Richard.L
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
