lkml.org 
[lkml]   [2017]   [Oct]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH 2/9 v2] usb: usb251xb: Add USB251x specific port count setting
    From
    Date

    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@gmail.com>
    > ---
    > 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

    \
     
     \ /
      Last update: 2017-10-04 09:37    [W:4.139 / U:0.888 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site