From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pf1-f177.google.com (mail-pf1-f177.google.com [209.85.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 50AC342A82 for ; Thu, 5 Feb 2026 15:01:43 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.210.177 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1770303703; cv=none; b=SBJdJ2zEE2N7tHVVSkh2DhjMbwraphARyXbtPGDyS0jbLvtwih5wacERqw+1y+Ju/mRCOc/120C3mIK7kpjEKf0TCB66Jn88/JwZaaiuiUqcBP7n6F4CYS13L8dcyPLsG9QluUXqShXkKdj5D6p57WnyQW+mSihoywQtUGLz0xg= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1770303703; c=relaxed/simple; bh=Cm2aEyQZvda4+HYwyRqk+YYubcQA9JycWksf8VhfyNQ=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=oF/FEOXUawkqEOMLY/5a57Hmc3BcI9ijZZ3v3V6/QuE53yXrQcFd77GTMVOQ2MUw300mAdmlLT28u1qntXDlqJIOx4Q+D8YeM257nXDtn2NZmqXzWlu57gshqEpetRMXUzVbkfTpslima77saYl7v3XXrJu3dkzONAod6QIhiM4= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=roeck-us.net; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=b4tMzgJo; arc=none smtp.client-ip=209.85.210.177 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=roeck-us.net Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="b4tMzgJo" Received: by mail-pf1-f177.google.com with SMTP id d2e1a72fcca58-81e8a9d521dso711113b3a.2 for ; Thu, 05 Feb 2026 07:01:43 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1770303703; x=1770908503; darn=vger.kernel.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:sender:from:to:cc:subject:date:message-id :reply-to; bh=sRi/5/Z2all2bs6QSSelNZ9jylVa1ACLnqIjRlhsrKk=; b=b4tMzgJo98WxfrPVHxTZkb9aF64YDLOLTd3PdJ9LlfPw4sKmhobC9dSj03nDWizsjj Swvl/ZTXYQ3i6UnyV5zEMvmvfY9TrnYyYxA8AwO22U0FO8EwgJZHEM0ru4JE3rx6YiW8 VWY8mMGch5jxrIxxnnjRdqZ/wWe/7BXVnjQyd8lXnug5R56LtL6I5YUvN31QUu0mSdwt tWGmgDt5ZWNNaDCNuyg1IXMUloDP/y6Ha77KCDtrr589Pnc9oROEoRef22jA4HJSsnTI Vw8MvFrbG6VessfY2eP0VGmukZL8Ke9VDcnc9PGyqNtH9sqPXy9RtHcNj2wy51JG+2T9 N7Mg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1770303703; x=1770908503; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:sender:x-gm-gg:x-gm-message-state:from:to :cc:subject:date:message-id:reply-to; bh=sRi/5/Z2all2bs6QSSelNZ9jylVa1ACLnqIjRlhsrKk=; b=jmW6RtnVdM9VG9im6cMNH7V6OyuzLH8YwMazZmcsRSXd4jLHi1qEKLZipzg3NErSX8 oJQZOVbp4toysbAHfgx0grUZ+NRD0F0zgMFC/DUcR0kh3X0hHNrO5Xe3BgwiHeWuBSNq 5pfh3ZKbdmmq/Lc1SywsTq/Vji0QhQz/2xpWF3zS7pGKJpdlUqzCulYyRSp2eA0Goyjd SZ2wjNTkrP+iwbdH4B6wyn4jxUSjsWmjSStw1q7P5Hei/DP5KwjnjkzlEkCQXOZhMsXT Wj6fOu8zDSwpNvs/3JHe9++04aDA+fDkprvnLiPvet5s9DgdPkv8YE3CnWtz34GmZ4zO M5OA== X-Forwarded-Encrypted: i=1; AJvYcCUT26CJbPUBkW1qfEl1ta9GxE9dSG3rM9lwe40oBlf1spurT6gCtXdnyJxl2OM25pd2+P0qSiHtdYnUnvY=@vger.kernel.org X-Gm-Message-State: AOJu0Yze3byoTuXnYwNdPbkHcjn4KPH8sKE7p5tf02wgysHOQ4BHxGqE cLu9vHC3QrwDj2Zs+kfBMl/RsnKUdB3H8qJUXA0E1jA2c+vGQ3XXvx06 X-Gm-Gg: AZuq6aLr3k1VPFA/d8YcVfqwgnPLnUxo4dxJoPYEytJ4WEbOtukcflfm3F8GRvmCcCz 6AHBIvjj7f/gMzwD6uX+kUJoxXmSf/ab5/LXzZQWWMrLgSckwdyz3z/Muec8v0xZTjqIWNKd9d3 7M4d/4VF1xWutV8s21gbTStEQ/510OPIPiWX59rFufx48pN83F17eOhILFxbuqyVuME1OJt4EaD IsCLQzHIh2NukudfiWHiuhPwRCaCq0yYvLkWnbGlVXPIyAt3td2cNgtf/u0JQyixYG0FGyinHgi vJ38lz5odvtHP9hNB8ZW5CMOtERtuKoQD7x5geYNiMoH3eut5x0ue4UIoBnJqjTcAHHiI5MzI/w L+fscyoHCNV3vqnf+sxJtNpFPo3wydEkI0qmVWZUc2V73CJFFAttMEslZq3eHDWybSHzlTbNBGT W5RqgNAnkZ0wL8MS7a/NoqJ4ov X-Received: by 2002:a05:6a00:12e2:b0:7e8:4471:ae74 with SMTP id d2e1a72fcca58-8241c7194a6mr6390068b3a.64.1770303702547; Thu, 05 Feb 2026 07:01:42 -0800 (PST) Received: from server.roeck-us.net ([2600:1700:e321:62f0:da43:aeff:fecc:bfd5]) by smtp.gmail.com with ESMTPSA id d2e1a72fcca58-8241d1c8c55sm6665929b3a.24.2026.02.05.07.01.41 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 05 Feb 2026 07:01:41 -0800 (PST) Sender: Guenter Roeck Date: Thu, 5 Feb 2026 07:01:40 -0800 From: Guenter Roeck To: Colin Huang Cc: Rob Herring , Krzysztof Kozlowski , Conor Dooley , Jonathan Corbet , linux-hwmon@vger.kernel.org, linux-kernel@vger.kernel.org, devicetree@vger.kernel.org, linux-doc@vger.kernel.org, Colin.Huang2@amd.com, Carl.Lee@amd.com, Peter.Shen@amd.com Subject: Re: [PATCH 1/3] hwmon: (pmbus) Add Delta Q54SN120A1 Q54SW120A7 driver Message-ID: References: <20260205-add-q54sn120a1-q54q54sw120a7-v1-0-09061ecacfc7@gmail.com> <20260205-add-q54sn120a1-q54q54sw120a7-v1-1-09061ecacfc7@gmail.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20260205-add-q54sn120a1-q54q54sw120a7-v1-1-09061ecacfc7@gmail.com> On Thu, Feb 05, 2026 at 09:34:35PM +0800, Colin Huang wrote: > Add the pmbus driver for DELTA Q54SN120A1, Q54SW120A7, > 1/4 Brick DC/DC Regulated Power Module with PMBus support > This isn't adding the driver, it is adding support for the chips to the q54sj108a2 driver. > Signed-off-by: Colin Huang > --- > drivers/hwmon/pmbus/q54sj108a2.c | 47 ++++++++++++++++++++++++++++++++++++---- > 1 file changed, 43 insertions(+), 4 deletions(-) > > diff --git a/drivers/hwmon/pmbus/q54sj108a2.c b/drivers/hwmon/pmbus/q54sj108a2.c > index 4d7086d83aa3..dca084c98fba 100644 > --- a/drivers/hwmon/pmbus/q54sj108a2.c > +++ b/drivers/hwmon/pmbus/q54sj108a2.c > @@ -21,7 +21,9 @@ > #define PMBUS_FLASH_KEY_WRITE 0xEC > > enum chips { > - q54sj108a2 > + q54sj108a2, > + q54sn120a1, > + q54sw120a7 > }; > > enum { > @@ -62,6 +64,34 @@ static struct pmbus_driver_info q54sj108a2_info[] = { > .format[PSC_VOLTAGE_IN] = linear, > .format[PSC_CURRENT_OUT] = linear, > > + .func[0] = PMBUS_HAVE_VIN | > + PMBUS_HAVE_VOUT | PMBUS_HAVE_STATUS_VOUT | > + PMBUS_HAVE_IOUT | PMBUS_HAVE_STATUS_IOUT | > + PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP | > + PMBUS_HAVE_STATUS_INPUT, > + }, > + [q54sn120a1] = { > + .pages = 1, > + > + /* Source : Delta Q54SN120A1 */ > + .format[PSC_TEMPERATURE] = linear, > + .format[PSC_VOLTAGE_IN] = linear, > + .format[PSC_CURRENT_OUT] = linear, > + > + .func[0] = PMBUS_HAVE_VIN | > + PMBUS_HAVE_VOUT | PMBUS_HAVE_STATUS_VOUT | > + PMBUS_HAVE_IOUT | PMBUS_HAVE_STATUS_IOUT | > + PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP | > + PMBUS_HAVE_STATUS_INPUT, > + }, > + [q54sw120a7] = { > + .pages = 1, > + > + /* Source : Delta Q54SW120A7 */ > + .format[PSC_TEMPERATURE] = linear, > + .format[PSC_VOLTAGE_IN] = linear, > + .format[PSC_CURRENT_OUT] = linear, > + Unless I am missing something, those are all the same. That means there is no need for separate entries. > .func[0] = PMBUS_HAVE_VIN | > PMBUS_HAVE_VOUT | PMBUS_HAVE_STATUS_VOUT | > PMBUS_HAVE_IOUT | PMBUS_HAVE_STATUS_IOUT | > @@ -269,6 +299,8 @@ static const struct file_operations q54sj108a2_fops = { > > static const struct i2c_device_id q54sj108a2_id[] = { > { "q54sj108a2", q54sj108a2 }, > + { "q54sn120a1", q54sn120a1 }, > + { "q54sw120a7", q54sw120a7 }, Delta sells a variety of power bricks, but a Google search for q54sn120a1 or q54sw120a7 comes up empty. Worse, searching for the entire series (q54sn or q54sw) comes up empty as well. Please provide information confirming that the referenced chips do exist. > { }, > }; > > @@ -278,6 +310,7 @@ static int q54sj108a2_probe(struct i2c_client *client) > { > struct device *dev = &client->dev; > u8 buf[I2C_SMBUS_BLOCK_MAX + 1]; > + const struct i2c_device_id *mid; > enum chips chip_id; > int ret, i; > struct dentry *debugfs; > @@ -314,8 +347,12 @@ static int q54sj108a2_probe(struct i2c_client *client) > dev_err(dev, "Failed to read Manufacturer Model\n"); > return ret; > } > - if (ret != 14 || strncmp(buf, "Q54SJ108A2", 10)) { > - buf[ret] = '\0'; > + buf[ret] = '\0'; > + for (mid = q54sj108a2_id; mid->name[0]; mid++) { > + if (!strncasecmp(mid->name, buf, strlen(mid->name))) > + break; > + } > + if (!mid->name[0]) { > dev_err(dev, "Unsupported Manufacturer Model '%s'\n", buf); > return -ENODEV; > } > @@ -325,7 +362,7 @@ static int q54sj108a2_probe(struct i2c_client *client) > dev_err(dev, "Failed to read Manufacturer Revision\n"); > return ret; > } > - if (ret != 4 || buf[0] != 'S') { > + if (buf[0] != 'S') { This relaxes the revision number checks significantly. Please provide evidence explaining why this is needed. > buf[ret] = '\0'; > dev_err(dev, "Unsupported Manufacturer Revision '%s'\n", buf); > return -ENODEV; > @@ -402,6 +439,8 @@ static int q54sj108a2_probe(struct i2c_client *client) > > static const struct of_device_id q54sj108a2_of_match[] = { > { .compatible = "delta,q54sj108a2", .data = (void *)q54sj108a2 }, > + { .compatible = "delta,q54sn120a1", .data = (void *)q54sn120a1 }, > + { .compatible = "delta,q54sw120a7", .data = (void *)q54sw120a7 }, > { }, > }; > > > -- > 2.34.1 >