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 32C4FC5478A for ; Wed, 21 Feb 2024 21:18:25 +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=O06JZBVYx125SKQmhubkV+hl9nio+YaltcTLyzjT2Po=; b=WMNrqysjgIiilQ /eAgS3umkdLu1reUki3dMq56HjDS1EXEc55dWNxNok8ThTpqpg1kgG+5VZOGTunDTIm/Ehu3OBT9R A7dVMZDXc6yfWagsn56jKFy4dnSR6a0lQXDNYnYn3vU1/BmFyUX7aNcz4PSS9HR70hB4KQyKnu30W 6s/jvnkqM2eB6ESGIiVLdKipdM9QcHpYcjYwouq7Udq3E0Dg0mLLQscyn/N57DOYuWT+PZ6nvlZcv GrtOSrWmAdEOmxtCtODRxM95v2iYc6moAzh+uXyC663g1RgyKIEO6a0kGYmkuv++pWCIyGhb1Vp0o hkE295YUM6F27N6XJf0A==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1rctyo-00000002bWX-3Lo2; Wed, 21 Feb 2024 21:18:14 +0000 Received: from mail-io1-xd29.google.com ([2607:f8b0:4864:20::d29]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1rctyl-00000002bVS-3q3R for linux-arm-kernel@lists.infradead.org; Wed, 21 Feb 2024 21:18:13 +0000 Received: by mail-io1-xd29.google.com with SMTP id ca18e2360f4ac-7c48e13e0d7so307941239f.1 for ; Wed, 21 Feb 2024 13:18:10 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; t=1708550290; x=1709155090; 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=M2o4Lt7sMW8yN4+uF0Im7dqQst5Gz28e1peL6R+UYnY=; b=ddXuZ+Vv2gndAwmb39ntabx/KWrb4IZOLwtnHErF4lfSYEA8wPr/3aSlnVualLvTf+ Enw6kSOeiAO4+9tlHzQvS789KdRe/cTOLdeYa7bOQsQRZwTb/SFJruP7FOeWmJ82jVOr LximT9noLBvoqEtnjv4mL4lqKVBwFzJi0QItc= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1708550290; x=1709155090; 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=M2o4Lt7sMW8yN4+uF0Im7dqQst5Gz28e1peL6R+UYnY=; b=Md17kEyxAbZeiNB8X+Nkh0EAAsnNBdLMS2tSDgDPYdStrWh3js0U5gkaB6U+eyIpSc +n/jncGw4wv50Ssf9hymcJh7f1/kMv4NGb7nCA2CoIgrD8Qtq3I/IrCz5Chs2t2jUR1v idvZY/nDxO8Nhl88mdwytJajfbkIFyjj7lNnIZP1LBSPwuW2RdjZ9ToqeSahmOzMVCtO tfWcblS2U5Ol0ixJoU7kR4lR6SCLrflQWf6RtrXsC/f+/jBVyZvzw7E2OU9ymTSKkrl+ BLFyxXas6H3+tX01mkj9siPJwWtIjuhOgiaoBOVI7SNvxWaKtWjpnSqTE1UVzApTzVWK kwxQ== X-Forwarded-Encrypted: i=1; AJvYcCWHTMCT5qv1nTVBpAOyzHI7DLTHH3MEN4LPrb08SM6MNAz1js4KhjnYuOQLFnzbNChDVJriGd+yMio4Rdxvr1+NLSYKWz3u8E3A1aBE90A4FPxB0uA= X-Gm-Message-State: AOJu0YxMqehuOd5TVQ4N1FBlukCOBJnf7dX2VtK8khLLwSdgqaS603OE hcGR7hv8ySt7GsxDrYt65WE74lVnkSgLgbERtsq1dRKNMbCOwXmHcrmfNTIpJQ== X-Google-Smtp-Source: AGHT+IFCEkFAfp0WCqH40+qAFwemcqmk5bthCRNNsiVwbLdDrdKoCN24aa7ZKBsYLJkAuxIEgXFMUA== X-Received: by 2002:a6b:c90d:0:b0:7c4:8b24:f926 with SMTP id z13-20020a6bc90d000000b007c48b24f926mr21213879iof.6.1708550290037; Wed, 21 Feb 2024 13:18:10 -0800 (PST) Received: from localhost (147.220.222.35.bc.googleusercontent.com. [35.222.220.147]) by smtp.gmail.com with UTF8SMTPSA id j3-20020a05663822c300b00472f79e0001sm2822585jat.171.2024.02.21.13.18.09 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 21 Feb 2024 13:18:09 -0800 (PST) Date: Wed, 21 Feb 2024 21:18:09 +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 v4 6/8] usb: misc: onboard_dev: use device supply names Message-ID: References: <20240220-onboard_xvf3500-v4-0-dc1617cc5dd4@wolfvision.net> <20240220-onboard_xvf3500-v4-6-dc1617cc5dd4@wolfvision.net> <503d9ea9-9812-498b-a5ee-2579ba8a7ecf@wolfvision.net> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <503d9ea9-9812-498b-a5ee-2579ba8a7ecf@wolfvision.net> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240221_131811_998841_E1530254 X-CRM114-Status: GOOD ( 41.59 ) 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, On Wed, Feb 21, 2024 at 09:40:38PM +0100, Javier Carrasco wrote: > On 21.02.24 21:25, Matthias Kaehlcke wrote: > > On Tue, Feb 20, 2024 at 03:05:50PM +0100, Javier Carrasco wrote: > >> The current mechanism 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 as a fallback mechanism for backward compatibility. > >> > >> Signed-off-by: Javier Carrasco > >> --- > >> drivers/usb/misc/onboard_usb_dev.c | 54 ++++++++++++++++++++------------------ > >> drivers/usb/misc/onboard_usb_dev.h | 23 ++++++++++++++++ > >> 2 files changed, 52 insertions(+), 25 deletions(-) > >> > >> diff --git a/drivers/usb/misc/onboard_usb_dev.c b/drivers/usb/misc/onboard_usb_dev.c > >> index f43130a6786f..e66fcac93006 100644 > >> --- a/drivers/usb/misc/onboard_usb_dev.c > >> +++ b/drivers/usb/misc/onboard_usb_dev.c > >> @@ -29,18 +29,6 @@ > >> > >> #include "onboard_usb_dev.h" > >> > >> -/* > >> - * Use generic names, as the actual names might differ between devices. If a new > >> - * device 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_dev_attach_usb_driver(struct work_struct *work); > >> > >> static struct usb_device_driver onboard_dev_usbdev_driver; > >> @@ -66,6 +54,33 @@ struct onboard_dev { > >> struct clk *clk; > >> }; > >> > >> +static int onboard_dev_get_regulator_bulk(struct device *dev, > >> + struct onboard_dev *onboard_dev) > >> +{ > >> + unsigned int i; > >> + int err; > >> + > >> + const char * const *supply_names = onboard_dev->pdata->supply_names; > >> + > >> + if (onboard_dev->pdata->num_supplies > MAX_SUPPLIES) > >> + return dev_err_probe(dev, -EINVAL, "max %zu supplies supported!\n", > >> + MAX_SUPPLIES); > >> + > >> + if (!supply_names[0]) > >> + supply_names = generic_supply_names; > > > > Please change to 'if (!supply_names)' and omit the initialization of > > .supply_names for devices that use the generic supply names. > > > > That looks much cleaner, I will apply it to the next version. > > >> diff --git a/drivers/usb/misc/onboard_usb_dev.h b/drivers/usb/misc/onboard_usb_dev.h > >> index ebe83e19d818..59dced6bd339 100644 > >> --- a/drivers/usb/misc/onboard_usb_dev.h > >> +++ b/drivers/usb/misc/onboard_usb_dev.h > >> @@ -6,63 +6,86 @@ > >> #ifndef _USB_MISC_ONBOARD_USB_DEV_H > >> #define _USB_MISC_ONBOARD_USB_DEV_H > >> > >> +/* > >> + * Fallback supply names for backwards compatibility. If the device requires > >> + * more than the currently supported supplies, add a new one here, and if > >> + * possible, the real name supplies to the device-specific data. > >> + */ > >> +static const char * const generic_supply_names[] = { > >> + "vdd", > >> + "vdd2", > >> +}; > >> + > >> +#define MAX_SUPPLIES ARRAY_SIZE(generic_supply_names) > > > > This will have to change when support for a device with more than 2 non-generic > > supply names gets added. Please use a literal value for MAX_SUPPLIES instead of > > ARRAY_SIZE. If the literal is 2 it would still need to change for future devices > > with more supplies, but that change would be more straighforward. > > > > I am not completely sure about this. Someone could increase MAX_SUPPLIES > without adding a generic name. That's perfectly fine and intended. MAX_SUPPLIES is a max, any list shorther than that is valid. Any longer list will result in probe() being aborted with a clear error message. > Actually two modifications will be necessary for every addition (name > and MAX_SUPPLIES). If ARRAY_SIZE is used, only new names are required, > and MAX_SUPPLIES is automatically increased. As per above it's not necessary to add a new name when MAX_SUPPLIES is increased to support more non-generic names. It would only be necessary if more generic names were added, my understanding is that this should not happen because any newly supported onboard devices are supposed to use device specific supply names. I don't like to idea of adding unused pseudo supply names to the list, just for the sake of using ARRAY_SIZE. > I understand that the whole point of this is getting rid of the generic > names, but we still have to provide generic names for every extra > supply, at least for code consistency and to avoid size mismatches > between real an generic supply names. Please let me know if you still think the extra names are needed. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel