From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.3 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE, SPF_PASS,URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 22AC6C2D0A8 for ; Mon, 28 Sep 2020 18:48:01 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id EE1C020BED for ; Mon, 28 Sep 2020 18:48:00 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726705AbgI1SsA (ORCPT ); Mon, 28 Sep 2020 14:48:00 -0400 Received: from netrider.rowland.org ([192.131.102.5]:48115 "HELO netrider.rowland.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with SMTP id S1726590AbgI1SsA (ORCPT ); Mon, 28 Sep 2020 14:48:00 -0400 Received: (qmail 145380 invoked by uid 1000); 28 Sep 2020 14:47:59 -0400 Date: Mon, 28 Sep 2020 14:47:59 -0400 From: Alan Stern To: Matthias Kaehlcke Cc: Greg Kroah-Hartman , Rob Herring , Frank Rowand , linux-kernel@vger.kernel.org, linux-usb@vger.kernel.org, Bastien Nocera , Stephen Boyd , Douglas Anderson , Ravi Chandra Sadineni , Krzysztof Kozlowski , devicetree@vger.kernel.org, Peter Chen , "Alexander A. Klimov" , "David S. Miller" , Johan Hovold , Masahiro Yamada , Mauro Carvalho Chehab , Rob Herring Subject: Re: [PATCH v4 2/2] USB: misc: Add onboard_usb_hub driver Message-ID: <20200928184759.GB142254@rowland.harvard.edu> References: <20200928101326.v4.1.I248292623d3d0f6a4f0c5bc58478ca3c0062b49a@changeid> <20200928101326.v4.2.I7c9a1f1d6ced41dd8310e8a03da666a32364e790@changeid> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20200928101326.v4.2.I7c9a1f1d6ced41dd8310e8a03da666a32364e790@changeid> User-Agent: Mutt/1.10.1 (2018-07-13) Precedence: bulk List-ID: X-Mailing-List: devicetree@vger.kernel.org On Mon, Sep 28, 2020 at 10:13:55AM -0700, Matthias Kaehlcke wrote: > The main issue this driver addresses is that a USB hub needs to be > powered before it can be discovered. For discrete onboard hubs (an > example for such a hub is the Realtek RTS5411) this is often solved > by supplying the hub with an 'always-on' regulator, which is kind > of a hack. Some onboard hubs may require further initialization > steps, like changing the state of a GPIO or enabling a clock, which > requires even more hacks. This driver creates a platform device > representing the hub which performs the necessary initialization. > Currently it only supports switching on a single regulator, support > for multiple regulators or other actions can be added as needed. > Different initialization sequences can be supported based on the > compatible string. > > Besides performing the initialization the driver can be configured > to power the hub off during system suspend. This can help to extend > battery life on battery powered devices which have no requirements > to keep the hub powered during suspend. The driver can also be > configured to leave the hub powered when a wakeup capable USB device > is connected when suspending, and power it off otherwise. > > Technically the driver consists of two drivers, the platform driver > described above and a very thin USB driver that subclasses the > generic driver. The purpose of this driver is to provide the platform > driver with the USB devices corresponding to the hub(s) (a hub > controller may provide multiple 'logical' hubs, e.g. one to support > USB 2.0 and another for USB 3.x). > > Co-developed-by: Ravi Chandra Sadineni > Signed-off-by: Ravi Chandra Sadineni > Signed-off-by: Matthias Kaehlcke > --- Minor cut & paste error: > +static int onboard_hub_power_off(struct onboard_hub *hub) > +{ > + int err; > + > + err = regulator_disable(hub->vdd); > + if (err) { > + dev_err(hub->dev, "failed to enable regulator: %d\n", err); s/enable/disable/ Have you tried manually unbinding and rebinding the two drivers a few times to make sure they will still work? I'm a little concerned about all the devm_* stuff in here; does that get released when the driver is unbound from the device or when the device is unregistered? And if the latter, what happens if you have multiple sysfs attribute groups going at the same time? Apart from those worries and the typo, this looks pretty good to me. Acked-by: Alan Stern Alan Stern