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 Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id B3DEAC47DD9 for ; Wed, 28 Feb 2024 15:37:35 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=2U8ZjQQTpqN1ezt2C0dey5/Z7uyFBUeyxLZMVRsJas4=; b=Cvf/gbfymBRHE6 dmQeKi2CV4eVNVkZLM0zC/Ge4L/BwfbwdiMEIQrUYKWdz+j6RT2x6ytPszcz2HC67SoMeF9+cbFaK 16pO6iNNiWsG6g37MdQAp+/oKjbeZHJg5uPpcVIUrWIootjHB08ELTD+QPWYpmotcRJee+lBfXGrt cA+FwQbnCbZsCQRANRGFqPpWvl0EKl/k7UMDjbfDCR8luWI0/bZE9eZflSD0vTWjU2DCNJfzAreCc wLN4Zk0sbD25cAwFu2hmgdwJcg50cFclu9f2MO9kNmUzIudKgj3xcvn+xBl0IoBXMpprCBaBkFBVR USe+MS27A3x0+g0WHi2Q==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1rfLzk-00000009v05-2iSb; Wed, 28 Feb 2024 15:37:20 +0000 Received: from mail-il1-x12a.google.com ([2607:f8b0:4864:20::12a]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1rfLzi-00000009uyx-2Hk7 for linux-arm-kernel@lists.infradead.org; Wed, 28 Feb 2024 15:37:19 +0000 Received: by mail-il1-x12a.google.com with SMTP id e9e14a558f8ab-3657d1d4516so19961585ab.2 for ; Wed, 28 Feb 2024 07:37:17 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; t=1709134637; x=1709739437; darn=lists.infradead.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=svD1W1a7QFf9Gza+G3SPXvK0VpFKMCR1+Xi0Z1FWhGs=; b=YVaSA4129qebKwdqcnnO3s7WfryHaw806VNubcpnuYU2GKNSvhTA4T8W2sj9q2bwzl DqhPlGm0ffxC1xx7oMiFMDAvWw6klIsYa0nT/1fTFqun8knRqanmeLKCp9cFNW3lPW/D 6F6C8wHv6dXzJFjVemzOgBK3RSiWGyFgeVpNU= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1709134637; x=1709739437; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=svD1W1a7QFf9Gza+G3SPXvK0VpFKMCR1+Xi0Z1FWhGs=; b=AG3O08qnxRfap2tov8M/pT3qflv57QmEkNLk+46+pwr2L+5lRVD2JLy9ch0yOL9Bh+ 31aKNxpiJqpmbwON13kahaED9x+O4t1xYCJr0/FsLD3ttmL0KtkWIfcWmA5wLFRTjrMD AbgxX7xZgfPcEEk6naac4xW7r40L4oQdBZcHoaHNHN5w9gPTAsYUebnHmy7hBS/+FuzJ XDi8K1TecEjH5Gh4zm5ZEMRKkUMFGim+NS69XF+7LZ5P4/pNB8nqnetTd9n24VBfg0RP AFLl8JUHCJ9kKvGf0r0QEg8h+jqMR1VJ94sXM5YaJuEWYzT98p6HTqk9fVYZYTKwLdBE 6Dkw== X-Forwarded-Encrypted: i=1; AJvYcCXWGof15IxXIvzNJxDmDIDrvnLSBuyDJDrS05MRAqe7wwAfTIuuANmdOPu4vEpGYjdyW3l+J+TbhbppyktHC4V+1x9vJNVNN1LclnViMW/AyIR/h34= X-Gm-Message-State: AOJu0YzVhl7QFFUgC9s+OPlwLwzOLomM9kIHUN6SNoCsx6U4nGWVyhRU 4ab/SMFi9O99LqWvthwxmWrq0ObTr/DHR/W8/6GLtZ84XQdcDuxY/BCFZFX6pA== X-Google-Smtp-Source: AGHT+IFwnHar0bSnDcMYJo9D3FZDjlUk0s+GVf8S+gT0uUdrPI6XVBO5n/eqspfTGLxTLggHdT93YQ== X-Received: by 2002:a92:de43:0:b0:365:1e8f:1519 with SMTP id e3-20020a92de43000000b003651e8f1519mr13223206ilr.27.1709134637291; Wed, 28 Feb 2024 07:37:17 -0800 (PST) Received: from localhost (144.57.222.35.bc.googleusercontent.com. [35.222.57.144]) by smtp.gmail.com with UTF8SMTPSA id i6-20020a056e021b0600b00363ce0ac359sm2766041ilv.47.2024.02.28.07.37.16 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 28 Feb 2024 07:37:16 -0800 (PST) Date: Wed, 28 Feb 2024 15:37:16 +0000 From: Matthias Kaehlcke To: Javier Carrasco Cc: Liam Girdwood , Mark Brown , Rob Herring , Krzysztof Kozlowski , Conor Dooley , Greg Kroah-Hartman , Helen Koike , Maarten Lankhorst , Maxime Ripard , Thomas Zimmermann , David Airlie , Daniel Vetter , Catalin Marinas , Will Deacon , Russell King , linux-sound@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-usb@vger.kernel.org, dri-devel@lists.freedesktop.org, linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH v5 1/8] usb: misc: onboard_hub: use device supply names Message-ID: References: <20240228-onboard_xvf3500-v5-0-76b805fd3fe6@wolfvision.net> <20240228-onboard_xvf3500-v5-1-76b805fd3fe6@wolfvision.net> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20240228-onboard_xvf3500-v5-1-76b805fd3fe6@wolfvision.net> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240228_073718_636716_87AE0588 X-CRM114-Status: GOOD ( 30.82 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Hi Javier, Thanks for moving this patch to the front of the series! A few more comments inline. On Wed, Feb 28, 2024 at 02:51:28PM +0100, Javier Carrasco wrote: > The current implementation uses generic names for the power supplies, > which conflicts with proper name definitions in the device bindings. > > Add a per-device property to include real supply names and keep generic > names for existing devices to keep backward compatibility. > > Signed-off-by: Javier Carrasco > --- > drivers/usb/misc/onboard_usb_hub.c | 49 ++++++++++++++++++++------------------ > drivers/usb/misc/onboard_usb_hub.h | 12 ++++++++++ > 2 files changed, 38 insertions(+), 23 deletions(-) > > diff --git a/drivers/usb/misc/onboard_usb_hub.c b/drivers/usb/misc/onboard_usb_hub.c > index 0dd2b032c90b..3755f6cc1eda 100644 > --- a/drivers/usb/misc/onboard_usb_hub.c > +++ b/drivers/usb/misc/onboard_usb_hub.c > @@ -29,17 +29,6 @@ > > #include "onboard_usb_hub.h" > > -/* > - * Use generic names, as the actual names might differ between hubs. If a new > - * hub requires more than the currently supported supplies, add a new one here. > - */ > -static const char * const supply_names[] = { > - "vdd", > - "vdd2", > -}; > - > -#define MAX_SUPPLIES ARRAY_SIZE(supply_names) > - > static void onboard_hub_attach_usb_driver(struct work_struct *work); > > static struct usb_device_driver onboard_hub_usbdev_driver; > @@ -65,6 +54,30 @@ struct onboard_hub { > struct clk *clk; > }; > > +static int onboard_hub_get_regulator_bulk(struct device *dev, > + struct onboard_hub *onboard_hub) Let's call this onboard_hub_get_regulators(), it's an implementation detail that regulator_bulk_get() is used for getting them. no need to pass 'dev', there is onboard_hub->dev > static int onboard_hub_power_on(struct onboard_hub *hub) > { > int err; > @@ -253,7 +266,6 @@ static int onboard_hub_probe(struct platform_device *pdev) > { > struct device *dev = &pdev->dev; > struct onboard_hub *hub; > - unsigned int i; > int err; > > hub = devm_kzalloc(dev, sizeof(*hub), GFP_KERNEL); > @@ -264,18 +276,9 @@ static int onboard_hub_probe(struct platform_device *pdev) > if (!hub->pdata) > return -EINVAL; > > - if (hub->pdata->num_supplies > MAX_SUPPLIES) > - return dev_err_probe(dev, -EINVAL, "max %zu supplies supported!\n", > - MAX_SUPPLIES); > - > - for (i = 0; i < hub->pdata->num_supplies; i++) > - hub->supplies[i].supply = supply_names[i]; > - > - err = devm_regulator_bulk_get(dev, hub->pdata->num_supplies, hub->supplies); > - if (err) { > - dev_err(dev, "Failed to get regulator supplies: %pe\n", ERR_PTR(err)); > + err = onboard_hub_get_regulator_bulk(dev, onboard_hub); The local variable is called 'hub', not 'onboard_hub'. > diff --git a/drivers/usb/misc/onboard_usb_hub.h b/drivers/usb/misc/onboard_usb_hub.h > index f360d5cf8d8a..ea24bd6790f0 100644 > --- a/drivers/usb/misc/onboard_usb_hub.h > +++ b/drivers/usb/misc/onboard_usb_hub.h > @@ -6,54 +6,66 @@ > #ifndef _USB_MISC_ONBOARD_USB_HUB_H > #define _USB_MISC_ONBOARD_USB_HUB_H > > +#define MAX_SUPPLIES 2 > + > struct onboard_hub_pdata { > unsigned long reset_us; /* reset pulse width in us */ > unsigned int num_supplies; /* number of supplies */ > + const char * const supply_names[MAX_SUPPLIES]; /* use the real names */ The comment isn't particularly useful or accurate. Not in all cases real names are used and outside of the context of this change the comment is hard to understand. I'd say just omit it, the name of the field is self-documenting enough, there is no need to repeat the same in a comment (as for 'num_supplies' ...) _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel