* Re: [PATCH 07/12] rtc: rzn1: fix alarm range check truncation on 32-bit systems
From: Wolfram Sang @ 2026-06-23 21:08 UTC (permalink / raw)
To: Lad, Prabhakar
Cc: Miquel Raynal, Alexandre Belloni, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Geert Uytterhoeven,
Magnus Damm, linux-rtc, linux-renesas-soc, devicetree,
linux-kernel, Biju Das, Fabrizio Castro, Lad Prabhakar
In-Reply-To: <CA+V-a8u2wt6623mYjhipOvJPo4va+bXs3qirQewocFr2QmUFhA@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 423 bytes --]
> Can you please share the commands you tried, I'll try and replicate it
> on my side.
Sorry, can't give you the commands, just from my head: I tried to set an
alarm more than a week in the future, and the alarm was set to the next
day. But I was in a hurry, maybe I overlooked something, because that
handling used to work in the past IIRC. I can return to this topic on
Friday earliest, sadly. Maybe next week only...
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH 2/3] dt-bindings: hwmon: pmbus: Support for onsemi's FD5121
From: Conor Dooley @ 2026-06-23 21:14 UTC (permalink / raw)
To: Selvamani Rajagopal
Cc: Guenter Roeck, Jonathan Corbet, Shuah Khan, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, linux-hwmon@vger.kernel.org,
linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
devicetree@vger.kernel.org
In-Reply-To: <CYYPR02MB98280DF78A07EADACFD084EE83EE2@CYYPR02MB9828.namprd02.prod.outlook.com>
[-- Attachment #1: Type: text/plain, Size: 1717 bytes --]
On Tue, Jun 23, 2026 at 09:01:32PM +0000, Selvamani Rajagopal wrote:
>
> > -----Original Message-----
> > From: Conor Dooley <conor@kernel.org>
> > Subject: Re: [PATCH 2/3] dt-bindings: hwmon: pmbus: Support for onsemi's FD5121
> >
> >
> > > +
> > > +title: onsemi's multi-phase digital controllers
> >
> > Can someone explain to me what a "digital controller" actually is?
> > Seems very generi and that a word may have been left out, were it not
> > for the fact that this wording is used several times in the patch.
> >
>
> Thanks for reviewing.
>
> According to me, "digital controller" means the controller uses digital circuits to implement
> the features and functionality. We can remove "digital" and keep only controller. It won't make any
> difference for Linux documentation.
My point is that what's actually being controlled is missing. Maybe it
is obvious to you, but it is not to me. Your nodename in your example is
> + fd5121@50 {
which doesn't comply with node naming requirements and I wanted to come
up with a suggestion for what it should be.
I am assuming that its power or voltage that you're controlling so
either it should be hwmon@ or regulator@.
>
> > > +
> > > + enum:
> > > + - onnn,fd5121
> > > + - onnn,fd5123
> > > + - onnn,fd5125
> >
> > Your /OF/ match data in your driver suggests that you intended to permit
> > fallback compatibles here?
>
> Agree. Sorry about the discrepancy. Will fix it.
>
> >
> > |+#ifdef CONFIG_OF
> > |+static const struct of_device_id fd5121_of_match[] = {
> > |+ { .compatible = "onnn,fd5121" },
> > |+ { }
> > |+};
> > |+MODULE_DEVICE_TABLE(of, fd5121_of_match);
> > |+#endif
> >
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH v5 1/3] dt-bindings: iio: health: add adi,max86150
From: sashiko-bot @ 2026-06-23 21:17 UTC (permalink / raw)
To: Md Shofiqul Islam; +Cc: devicetree, robh, conor+dt
In-Reply-To: <20260623201124.18271-2-shofiqtest@gmail.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The description for the `interrupts` property explicitly states it is an 'Active-low interrupt line', but the example uses `IRQ_TYPE_EDGE_FALLING`.
--
commit 842884bb765a65ccd68dc98c1bab3a30ca22640a
Author: Md Shofiqul Islam <shofiqtest@gmail.com>
dt-bindings: iio: health: add adi,max86150
Add Device Tree binding schema for the Analog Devices MAX86150
integrated ECG and PPG biosensor.
> diff --git a/Documentation/devicetree/bindings/iio/health/adi,max86150.yaml b/Documentation/devicetree/bindings/iio/health/adi,max86150.yaml
> new file mode 100644
> index 0000000000000..c191f4f1525b1
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/health/adi,max86150.yaml
> @@ -0,0 +1,78 @@
[ ... ]
> + interrupts:
> + maxItems: 1
> + description: |
> + Active-low interrupt line. Asserted when the FIFO almost-full
> + threshold is reached or when a new PPG sample is ready.
[ ... ]
> +examples:
> + - |
> + #include <dt-bindings/interrupt-controller/irq.h>
> +
> + i2c {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + heart-rate@5e {
> + compatible = "adi,max86150";
> + reg = <0x5e>;
> + interrupt-parent = <&gpio1>;
> + interrupts = <5 IRQ_TYPE_EDGE_FALLING>;
[Severity: Low]
The property description states this is an "Active-low interrupt line",
which typically corresponds to IRQ_TYPE_LEVEL_LOW. Does the example use
IRQ_TYPE_EDGE_FALLING intentionally, or should it be updated to match the
description to avoid misleading developers who might copy this example?
> + vdd-supply = <&vdd_1v8>;
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260623201124.18271-1-shofiqtest@gmail.com?part=1
^ permalink raw reply
* Re: [PATCH v5 2/3] iio: health: add MAX86150 ECG and PPG biosensor driver
From: sashiko-bot @ 2026-06-23 21:30 UTC (permalink / raw)
To: Md Shofiqul Islam; +Cc: robh, conor+dt, devicetree
In-Reply-To: <20260623201124.18271-3-shofiqtest@gmail.com>
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Hardware FIFO overrun and data corruption due to incorrect burst read length.
- [High] Infinite IRQ storm on level-triggered interrupts due to improper IRQF_ONESHOT handling, and potential DoS via missing .validate_device.
- [Medium] IIO trigger reference leak on probe error path.
--
commit 9590a122a97de73064d5ca132b0611828fac557d
Author: Md Shofiqul Islam <shofiqtest@gmail.com>
iio: health: add MAX86150 ECG and PPG biosensor driver
Add a new IIO driver for the Analog Devices MAX86150 integrated
biosensor, which combines two PPG optical channels (Red/IR LED) and
one ECG biopotential channel in a single I2C device.
> diff --git a/drivers/iio/health/max86150.c b/drivers/iio/health/max86150.c
> new file mode 100644
> index 0000000000000..92bf31f0a7615
> --- /dev/null
> +++ b/drivers/iio/health/max86150.c
[ ... ]
> +static int max86150_read_one_sample(struct max86150_data *data,
> + u32 *ppg_red, u32 *ppg_ir, s32 *ecg)
> +{
> + int ret;
> +
> + /*
> + * Use data->fifo_raw (heap memory) not a local array so the buffer is
> + * DMA-mappable for I2C host controllers that use DMA for burst reads.
> + */
> + ret = regmap_noinc_read(data->regmap, MAX86150_REG_FIFO_DATA,
> + data->fifo_raw, sizeof(data->fifo_raw));
[Severity: High]
Since data->fifo_raw is padded to ARCH_DMA_MINALIGN (e.g., 64 or 128 bytes)
for DMA safety, will sizeof(data->fifo_raw) cause the I2C burst read to pop
64 or 128 bytes from the hardware FIFO instead of the intended 9 bytes?
Could this destroy the 3-byte slot alignment and permanently drop data?
> + if (ret)
> + return ret;
[ ... ]
> +static const struct iio_trigger_ops max86150_trigger_ops = {
> + .set_trigger_state = max86150_set_trigger_state,
> +};
[Severity: High]
Does the absence of .validate_device here allow external IIO devices to attach
to this trigger?
If they do attach, could that prevent the consumer thread from clearing this
device's interrupt, leading to a permanent IRQ storm?
[ ... ]
> + irq_trig = irq_get_trigger_type(client->irq);
> + if (!irq_trig)
> + irq_trig = IRQF_TRIGGER_FALLING;
> +
> + ret = devm_request_threaded_irq(&client->dev, client->irq,
> + NULL,
> + iio_trigger_generic_data_rdy_poll,
> + irq_trig | IRQF_ONESHOT,
> + "max86150", data->trig);
[Severity: High]
For level-triggered interrupts, will IRQF_ONESHOT correctly mask the line
until the hardware is cleared?
Looking at iio_trigger_generic_data_rdy_poll(), it acts as the primary
threaded handler and returns IRQ_HANDLED immediately after scheduling the
consumer thread.
Doesn't returning IRQ_HANDLED unmask the interrupt line before
max86150_trigger_handler() has a chance to read the hardware registers and
clear the source, causing an immediate IRQ storm?
> + if (ret)
> + return dev_err_probe(&client->dev, ret,
> + "Cannot request IRQ %d\n",
> + client->irq);
> +
> + ret = devm_iio_trigger_register(&client->dev, data->trig);
> + if (ret)
> + return dev_err_probe(&client->dev, ret,
> + "Cannot register trigger\n");
> +
> + /*
> + * Set the default trigger so userspace can enable the buffer
> + * without a manual current_trigger write. iio_device_unregister()
> + * calls iio_trigger_put() to release this reference.
> + */
> + indio_dev->trig = iio_trigger_get(data->trig);
> + }
> +
> + ret = devm_iio_triggered_buffer_setup(&client->dev, indio_dev,
> + iio_pollfunc_store_time,
> + max86150_trigger_handler,
> + NULL);
[Severity: Medium]
If devm_iio_triggered_buffer_setup() fails here, is the trigger reference
acquired by iio_trigger_get() leaked?
Since INDIO_BUFFER_TRIGGERED is not set when buffer setup fails,
iio_dev_release() will skip calling iio_device_unregister_trigger_consumer().
Does this mean iio_trigger_put() is never called to clean up this reference?
> + if (ret)
> + return dev_err_probe(&client->dev, ret,
> + "Cannot setup triggered buffer\n");
> +
> + return devm_iio_device_register(&client->dev, indio_dev);
> +}
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260623201124.18271-1-shofiqtest@gmail.com?part=2
^ permalink raw reply
* Re: [PATCH v3 3/3] iio: magnetometer: add driver for QST QMC5883L Sensor
From: Andy Shevchenko @ 2026-06-23 21:31 UTC (permalink / raw)
To: Siratul Islam
Cc: jic23, robh, krzk+dt, conor+dt, dlechner, nuno.sa, andy,
linux-iio, devicetree, linux-kernel
In-Reply-To: <20260619104524.10172-4-siratul.islam@linux.dev>
On Fri, Jun 19, 2026 at 04:45:06PM +0600, Siratul Islam wrote:
> Add driver for the QST QMC5883L 3-Axis Magnetic Sensor
> connected via i2c.
LGTM now,
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v2 3/4] drm/ssd130x: Add SSD135X_FAMILY and SSD1351 support
From: Amit Barzilai @ 2026-06-23 21:34 UTC (permalink / raw)
To: markus.elfring
Cc: airlied, amit.barzilai22, andy, azuddinadam, chintanlike,
conor+dt, deller, devicetree, dri-devel, gregkh, javierm, krzk+dt,
linux-fbdev, linux-kernel, linux-staging, maarten.lankhorst,
mripard, robh, simona, tzimmermann
In-Reply-To: <16a86f3d-caf0-46d0-97a4-c9585bdaa06c@web.de>
Markus Elfring <Markus.Elfring@web.de> writes:
>> +++ b/drivers/gpu/drm/solomon/ssd130x.c
>> @@ -146,6 +146,33 @@
>> #define SSD133X_COLOR_DEPTH_256 0x0
>> #define SSD133X_COLOR_DEPTH_65K 0x1
>>
>> +/* ssd135x commands */
>> +#define SSD135X_SET_COL_RANGE 0x15
>> +#define SSD135X_WRITE_RAM 0x5c
>> +#define SSD135X_SET_ROW_RANGE 0x75
> [...]
>
> How do you think about to use an enumeration for such data?
> https://en.wikipedia.org/wiki/Enumerated_type#C_and_syntactically_similar_languages
Thank you for the suggestion.
I used #define to stay consistent with the rest of ssd130x.c, where the
command constants for the other families are all defined the same way.
In my opinion an enum could be a readable solution for these values, but I
don't think the switch should be included in this series.
--
Thanks,
Amit
^ permalink raw reply
* Re: [PATCH v2 5/5] riscv: dts: spacemit: improve RTL8211F PHY configuration on K3 Pico-ITX board
From: sashiko-bot @ 2026-06-23 21:39 UTC (permalink / raw)
To: Aurelien Jarno; +Cc: devicetree, robh, conor+dt
In-Reply-To: <20260623204431.498700-6-aurelien@aurel32.net>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Adding vendor-specific properties (`realtek,aldps-enable` and `realtek,clkout-disable`) to a PHY node with a generic `compatible` string bypasses DT schema validation and perpetuates probing fragility.
--
commit 06c0a01ebf26666aceea3d45a75bbed3025600af
Author: Aurelien Jarno <aurelien@aurel32.net>
riscv: dts: spacemit: improve RTL8211F PHY configuration on K3 Pico-ITX board
This commit enables ALDPS and disables the clkout signal on the RTL8211F
PHY for the K3 Pico-ITX board to save power and match hardware schematics.
> diff --git a/arch/riscv/boot/dts/spacemit/k3-pico-itx.dts b/arch/riscv/boot/dts/spacemit/k3-pico-itx.dts
> index 532c72006ef1f..2476f5585a200 100644
> --- a/arch/riscv/boot/dts/spacemit/k3-pico-itx.dts
> +++ b/arch/riscv/boot/dts/spacemit/k3-pico-itx.dts
> @@ -226,6 +226,8 @@ phy0: phy@1 {
> reset-gpios = <&gpio 0 15 GPIO_ACTIVE_LOW>;
> reset-assert-us = <10000>;
> reset-deassert-us = <10000>;
> + realtek,aldps-enable;
> + realtek,clkout-disable;
[Severity: Medium]
Will adding these vendor-specific properties without specifying a Realtek
PHY ID in the compatible string bypass device tree schema validation?
The realtek,rtl82xx.yaml schema is only selected if a Realtek PHY ID is
present. Since the phy0 node only specifies ethernet-phy-ieee802.3-c22, the
validation tools will ignore the new Realtek-specific properties, meaning
typos or invalid types could pass silently during dtbs_check.
Also, since this node uses reset-gpios, if the PHY is kept in reset by
hardware or the bootloader, could the MDIO framework fail to read its ID
and probe it unless the exact PHY ID is provided in the compatible string?
Would it be better to update the compatible string to include the specific
PHY ID (such as ethernet-phy-id001c.c916) alongside the generic one to
ensure proper schema validation and reliable probing?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260623204431.498700-1-aurelien@aurel32.net?part=5
^ permalink raw reply
* Re: [PATCH RFC] dt-bindings: sound: Convert cirrus,cs35l36 to DT schema
From: Rhodes, David @ 2026-06-23 22:00 UTC (permalink / raw)
To: David Heidelberg, David Rhodes, Richard Fitzgerald, Liam Girdwood,
Mark Brown, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
patches, Bjorn Helgaas
Cc: linux-sound, devicetree, linux-kernel, phone-devel
In-Reply-To: <513dd76c-586e-48e3-8542-5f42d35c7929@ixit.cz>
On 6/21/26 7:27 AM, David Heidelberg wrote:
> On 18/06/2026 23:38, Rhodes, David wrote:
>> On 6/18/26 9:33 AM, David Heidelberg via B4 Relay wrote:
>>> From: David Heidelberg <david@ixit.cz>
>>>
>>> Convert CS35L36 Speaker Amplifier.
>>>
>>> Changes:
>>> - maintainers email to the generic Cirrus email
>>> - Both the codec and downstream worked just fine without
>>> VP-supply provided. Align with datasheet for similar models.
>>> - add dai-common.yaml to cover for '#sound-dai-cells',
>>> 'sound-name-prefix'
>>>
>>> Co-developed-by: Rob Herring (Arm) <robh@kernel.org>
>>> Signed-off-by: Rob Herring (Arm) <robh@kernel.org>
>>> Signed-off-by: David Heidelberg <david@ixit.cz>
>>> ---
>>
>> Reviewed-by: David Rhodes <David.Rhodes@cirrus.com>
>
> Thanks for the review. btw. could you please verify against datasheet,
> if the
>
> - cirrus,classh-wk-fet-delay is also possible for cs35l36 (l35 has it,
> but l36 driver doesn't, thou DT isn't about driver, so checking)
> - cirrus,weak-fet-disable and cirrus,classh-weak-fet-thld-millivolt
> are also not implemented in any driver, but present in DT.
>
> If hardware is capable and the driver just doesn't have impl. it's fine,
> but otherwise I would remove these in v2.
>
> Thank you
> David :D
>
Hardware is capable and the driver does not have implementation.
This would be the register referred to as CS35L36_CLASSH_FET_DRV_CFG.
Thanks,
David
^ permalink raw reply
* Re: [PATCH 3/3] hwmon: (pmbus/fd5121): Add support FD5121, FD5123 and FD5125
From: Guenter Roeck @ 2026-06-23 22:07 UTC (permalink / raw)
To: Selvamani.Rajagopal, Jonathan Corbet, Shuah Khan, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: linux-hwmon, linux-doc, linux-kernel, devicetree
In-Reply-To: <20260622-support-fd5121-from-onsemi-v1-3-b31767689c65@onsemi.com>
On 6/22/26 22:55, Selvamani Rajagopal via B4 Relay wrote:
> From: Selvamani Rajagopal <Selvamani.Rajagopal@onsemi.com>
>
> FD5121 is a dual-rail, multi-phase, digital controller that offers
> full telemtry options including input/output voltage, current as
> well as fault handling and identifications.
>
> These controllers are compliant with PMBus specification.
>
> Signed-off-by: Selvamani Rajagopal <Selvamani.Rajagopal@onsemi.com>
> ---
> MAINTAINERS | 8 +
> drivers/hwmon/pmbus/Kconfig | 9 +
> drivers/hwmon/pmbus/Makefile | 1 +
> drivers/hwmon/pmbus/fd5121.c | 1004 ++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 1022 insertions(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index d95d3ef77773..c0664c33324a 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -20135,6 +20135,14 @@ L: linux-mips@vger.kernel.org
> S: Maintained
> F: arch/mips/boot/dts/ralink/omega2p.dts
>
> +ONSEMI HARDWARE MONITOR DRIVER
> +M: Selva Rajagopal <selvamani.rajagopal@onsemi.com>
> +L: linux-hwmon@vger.kernel.org
> +S: Supported
> +W: https://www.onsemi.com
> +F: Documentation/devicetree/bindings/hwmon/pmbus/onnn,fd5121.yaml
> +F: drivers/hwmon/pmbus/fd5121.c
> +
> ONSEMI ETHERNET PHY DRIVERS
> M: Piergiorgio Beruto <piergiorgio.beruto@gmail.com>
> L: netdev@vger.kernel.org
> diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig
> index c8cda160b5f8..3a06ed83539e 100644
> --- a/drivers/hwmon/pmbus/Kconfig
> +++ b/drivers/hwmon/pmbus/Kconfig
> @@ -179,6 +179,15 @@ config SENSORS_E50SN12051
> This driver can also be built as a module. If so, the module will
> be called e50sn12051.
>
> +config SENSORS_FD5121
> + tristate "FD5121/FD5123/FD5125 controllers from onsemi"
> + help
> + If you say yes here, you get support for onsemi
> + controllers FD5121, FD5123, FD5125.
> +
> + This driver can also be built as a module. If so, the module will
> + be called fd5121.
> +
> config SENSORS_INA233
> tristate "Texas Instruments INA233 and compatibles"
> help
> diff --git a/drivers/hwmon/pmbus/Makefile b/drivers/hwmon/pmbus/Makefile
> index ffc05f493213..70f4afb41fe0 100644
> --- a/drivers/hwmon/pmbus/Makefile
> +++ b/drivers/hwmon/pmbus/Makefile
> @@ -13,6 +13,7 @@ obj-$(CONFIG_SENSORS_APS_379) += aps-379.o
> obj-$(CONFIG_SENSORS_BEL_PFE) += bel-pfe.o
> obj-$(CONFIG_SENSORS_BPA_RS600) += bpa-rs600.o
> obj-$(CONFIG_SENSORS_DELTA_AHE50DC_FAN) += delta-ahe50dc-fan.o
> +obj-$(CONFIG_SENSORS_FD5121) += fd5121.o
> obj-$(CONFIG_SENSORS_FSP_3Y) += fsp-3y.o
> obj-$(CONFIG_SENSORS_HAC300S) += hac300s.o
> obj-$(CONFIG_SENSORS_IBM_CFFPS) += ibm-cffps.o
> diff --git a/drivers/hwmon/pmbus/fd5121.c b/drivers/hwmon/pmbus/fd5121.c
> new file mode 100644
> index 000000000000..e68c6d6cabbd
> --- /dev/null
> +++ b/drivers/hwmon/pmbus/fd5121.c
> @@ -0,0 +1,1004 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright 2026 Semiconductor Components Industries, LLC ("onsemi").
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/err.h>
> +#include <linux/slab.h>
> +#include <linux/delay.h>
> +#include <linux/i2c.h>
> +#include <linux/hwmon.h>
> +#include <linux/hwmon-sysfs.h>
> +#include <linux/unaligned.h>
> +
> +#include "pmbus.h"
> +
> +enum chips { chip_fd5121, chip_fd5123, chip_fd5125 };
> +
> +#define CTLR_ID_UNKNOWN 0
> +#define CTLR_ID_FD5121 0xFD5121
> +#define CTLR_ID_FD5123 0xFD5123
> +#define CTLR_ID_FD5125 0xFD5125
> +
> +#define FD5121_NUM_PAGES 2
> +
> +/* Custom PMBUS commands */
> +#define PMBUS_REG_VOUT_MIN 0x2B
> +#define PMBUS_REG_POWER_MODE 0x34
> +#define PMBUS_REG_VIN_ON 0x35
> +#define PMBUS_REG_VIN_OFF 0x36
> +#define PMBUS_REG_VIN_UV_FAULT_RESPONSE 0x5A
> +#define PMBUS_REG_IIN_OC_FAULT_RESPONSE 0x5C
> +#define PMBUS_REG_TON_DELAY 0x60
> +#define PMBUS_REG_POUT_OP_FAULT_RESPONSE 0x69
> +#define PMBUS_REG_READ_VAUX 0x85
> +
> +#define PMBUS_REG_IKNEE_SET 0x2D
> +#define PMBUS_REG_PIN_COUNTER 0x2E
> +#define PMBUS_REG_VMIN_AWARE 0x2F
> +#define PMBUS_REG_VAUX_UV_FAULT_LIMIT 0x6C
> +#define PMBUS_REG_VAUX_OV_FAULT_LIMIT 0x6D
> +#define PMBUS_REG_VAUX_UV_FAULT_RESPONSE 0x6E
> +#define PMBUS_REG_VAUX_OV_FAULT_RESPONSE 0x6F
> +#define PMBUS_REG_VAUX_UV_WARNING 0x75
> +#define PMBUS_REG_VAUX_OV_WARNING 0x76
> +#define PMBUS_REG_MFR_FREE_USER_CONFIG_TABLES 0xCF
> +#define PMBUS_REG_MFR_ADDRESS_TABLE 0xD0
> +#define PMBUS_REG_MFR_STATUS_ONSEMI 0xD1
> +#define PMBUS_REG_MFR_UNLOCK 0xD2
> +#define PMBUS_REG_MFR_FAULTY_SPS 0xD3
> +#define PMBUS_REG_TLVR_FAULTS 0xD4
> +#define PMBUS_REG_MFR_USER_STORE_CONFIG_TAB 0xD5
> +#define PMBUS_REG_MFR_USER_CONFIG_INDEX 0xD6
> +#define PMBUS_REG_MFR_PWM_DISCONNECTION 0xD7
> +#define PMBUS_REG_MFR_VR_DISCONNECTION 0xD8
> +#define PMBUS_REG_MFR_TON_SLEW 0xD9
> +#define PMBUS_REG_MFR_TOFF_SLEW 0xDA
> +#define PMBUS_REG_MFR_RAIL_NAME 0xDB
> +#define PMBUS_REG_MFR_VOUT_DROOP 0xDC
> +#define PMBUS_REG_MFR_USER_RESTORE_CONFIG_TAB 0xDD
> +#define PMBUS_REG_MFR_SVR_GO 0xDE
> +#define PMBUS_REG_MFR_SET_PWD 0xDF
> +#define PMBUS_REG_MFR_CONFIG_ACTIVATE 0xE0
> +#define PMBUS_REG_MFR_CONFIG_RECOVER 0xE1
> +#define PMBUS_REG_MFR_OTP_DUMP 0xE2
> +#define PMBUS_REG_MFR_BBR_EN 0xE3
> +#define PMBUS_REG_MFR_DPM_MIN 0xE4
> +#define PMBUS_REG_MFR_VBOOT 0xE5
> +#define PMBUS_REG_MFR_PRECLAMP_OFFSET 0xE6
> +#define PMBUS_REG_MFR_TLVR_DIAGN 0xE7
> +#define PMBUS_REG_MFR_READ_VSYS 0xE8
> +#define PMBUS_REG_MFR_SPECIFIC_E9 0xE9
> +#define PMBUS_REG_MFR_SPECIFIC_EA 0xEA
> +#define PMBUS_REG_MFR_SS_CBC 0xEB
> +#define PMBUS_REG_MFR_AMD_STATUS 0xEC
> +#define PMBUS_REG_MFR_CHECKSUM 0xEE
> +#define PMBUS_REG_CSE_INDEX 0xF0
> +#define PMBUS_REG_COUT_MEASURE 0xF1
> +#define PMBUS_REG_VR_COUT 0xF2
> +#define PMBUS_REG_BBR_RAM 0xF3
> +#define PMBUS_REG_BBR_OTP 0xF4
> +#define PMBUS_REG_READ_PSYS 0xF5
> +#define PMBUS_REG_POSTCLAMP_OFFSET 0xF6
> +#define PMBUS_REG_PGOOD_DELAY 0xF7
> +#define PMBUS_REG_MFR_SPECIFIC_F8 0xF8
> +#define PMBUS_REG_MFR_SPECIFIC_F9 0xF9
> +#define PMBUS_REG_MFR_PWD_PROGRAM_RAM 0xFA
> +#define PMBUS_REG_MFR_PWD_PROGRAM_I2C 0xFB
> +#define PMBUS_REG_MFR_PWD_ENABLE_OTP_STORE 0xFC
> +
> +/* List of recognized commands */
> +static const u8 cc_list[] = {
> + PMBUS_PAGE,
> + PMBUS_OPERATION,
> + PMBUS_ON_OFF_CONFIG,
> + PMBUS_CLEAR_FAULTS,
> + PMBUS_WRITE_PROTECT,
> + PMBUS_CAPABILITY,
> + PMBUS_VOUT_MODE,
> + PMBUS_VOUT_COMMAND,
> + PMBUS_VOUT_MAX,
> + PMBUS_VOUT_MARGIN_HIGH,
> + PMBUS_VOUT_MARGIN_LOW,
> + PMBUS_VOUT_TRANSITION_RATE,
> + PMBUS_REG_VOUT_MIN,
> + PMBUS_REG_IKNEE_SET,
> + PMBUS_REG_PIN_COUNTER,
> + PMBUS_REG_VMIN_AWARE,
> + PMBUS_REG_POWER_MODE,
> + PMBUS_REG_VIN_ON,
> + PMBUS_REG_VIN_OFF,
> + PMBUS_VOUT_OV_FAULT_LIMIT,
> + PMBUS_VOUT_OV_FAULT_RESPONSE,
> + PMBUS_VOUT_UV_FAULT_LIMIT,
> + PMBUS_VOUT_UV_FAULT_RESPONSE,
> + PMBUS_IOUT_OC_FAULT_LIMIT,
> + PMBUS_IOUT_OC_FAULT_RESPONSE,
> + PMBUS_IOUT_OC_WARN_LIMIT,
> + PMBUS_OT_FAULT_LIMIT,
> + PMBUS_OT_FAULT_RESPONSE,
> + PMBUS_OT_WARN_LIMIT,
> + PMBUS_VIN_OV_FAULT_LIMIT,
> + PMBUS_VIN_OV_FAULT_RESPONSE,
> + PMBUS_VIN_OV_WARN_LIMIT,
> + PMBUS_VIN_UV_WARN_LIMIT,
> + PMBUS_VIN_UV_FAULT_LIMIT,
> + PMBUS_REG_VIN_UV_FAULT_RESPONSE,
> + PMBUS_IIN_OC_FAULT_LIMIT,
> + PMBUS_REG_IIN_OC_FAULT_RESPONSE,
> + PMBUS_IIN_OC_WARN_LIMIT,
> + PMBUS_REG_TON_DELAY,
> + PMBUS_POUT_OP_FAULT_LIMIT,
> + PMBUS_REG_POUT_OP_FAULT_RESPONSE,
> + PMBUS_POUT_OP_WARN_LIMIT,
> + PMBUS_PIN_OP_WARN_LIMIT,
> + PMBUS_REG_VAUX_UV_FAULT_LIMIT,
> + PMBUS_REG_VAUX_OV_FAULT_LIMIT,
> + PMBUS_REG_VAUX_UV_FAULT_RESPONSE,
> + PMBUS_REG_VAUX_OV_FAULT_RESPONSE,
> + PMBUS_REG_VAUX_UV_WARNING,
> + PMBUS_REG_VAUX_OV_WARNING,
> + PMBUS_STATUS_BYTE,
> + PMBUS_STATUS_WORD,
> + PMBUS_STATUS_VOUT,
> + PMBUS_STATUS_IOUT,
> + PMBUS_STATUS_INPUT,
> + PMBUS_STATUS_TEMPERATURE,
> + PMBUS_STATUS_CML,
> + PMBUS_STATUS_OTHER,
> + PMBUS_STATUS_MFR_SPECIFIC,
> + PMBUS_REG_READ_VAUX,
> + PMBUS_READ_VIN,
> + PMBUS_READ_IIN,
> + PMBUS_READ_VOUT,
> + PMBUS_READ_IOUT,
> + PMBUS_READ_TEMPERATURE_1,
> + PMBUS_READ_POUT,
> + PMBUS_READ_PIN,
> + PMBUS_REVISION,
> + PMBUS_MFR_ID,
> + PMBUS_MFR_MODEL,
> + PMBUS_MFR_REVISION,
> + PMBUS_IC_DEVICE_ID,
> + PMBUS_REG_MFR_FREE_USER_CONFIG_TABLES,
> + PMBUS_REG_MFR_ADDRESS_TABLE,
> + PMBUS_REG_MFR_STATUS_ONSEMI,
> + PMBUS_REG_MFR_UNLOCK,
> + PMBUS_REG_MFR_FAULTY_SPS,
> + PMBUS_REG_TLVR_FAULTS,
> + PMBUS_REG_MFR_USER_STORE_CONFIG_TAB,
> + PMBUS_REG_MFR_USER_CONFIG_INDEX,
> + PMBUS_REG_MFR_PWM_DISCONNECTION,
> + PMBUS_REG_MFR_VR_DISCONNECTION,
> + PMBUS_REG_MFR_TON_SLEW,
> + PMBUS_REG_MFR_TOFF_SLEW,
> + PMBUS_REG_MFR_RAIL_NAME,
> + PMBUS_REG_MFR_VOUT_DROOP,
> + PMBUS_REG_MFR_USER_RESTORE_CONFIG_TAB,
> + PMBUS_REG_MFR_SVR_GO,
> + PMBUS_REG_MFR_SET_PWD,
> + PMBUS_REG_MFR_CONFIG_ACTIVATE,
> + PMBUS_REG_MFR_CONFIG_RECOVER,
> + PMBUS_REG_MFR_OTP_DUMP,
> + PMBUS_REG_MFR_BBR_EN,
> + PMBUS_REG_MFR_DPM_MIN,
> + PMBUS_REG_MFR_VBOOT,
> + PMBUS_REG_MFR_PRECLAMP_OFFSET,
> + PMBUS_REG_MFR_TLVR_DIAGN,
> + PMBUS_REG_MFR_READ_VSYS,
> + PMBUS_REG_MFR_SPECIFIC_E9,
> + PMBUS_REG_MFR_SPECIFIC_EA,
> + PMBUS_REG_MFR_SS_CBC,
> + PMBUS_REG_MFR_AMD_STATUS,
> + PMBUS_REG_MFR_CHECKSUM,
> + PMBUS_REG_CSE_INDEX,
> + PMBUS_REG_COUT_MEASURE,
> + PMBUS_REG_VR_COUT,
> + PMBUS_REG_BBR_RAM,
> + PMBUS_REG_BBR_OTP,
> + PMBUS_REG_READ_PSYS,
> + PMBUS_REG_POSTCLAMP_OFFSET,
> + PMBUS_REG_PGOOD_DELAY,
> + PMBUS_REG_MFR_SPECIFIC_F8,
> + PMBUS_REG_MFR_SPECIFIC_F9,
> + PMBUS_REG_MFR_PWD_PROGRAM_RAM,
> + PMBUS_REG_MFR_PWD_PROGRAM_I2C,
> + PMBUS_REG_MFR_PWD_ENABLE_OTP_STORE,
> +};
> +
> +/* Following registers expect block read */
> +static const u8 blk_rd_cc[] = {
> + PMBUS_SMBALERT_MASK,
> + PMBUS_MFR_DATE,
> + PMBUS_IC_DEVICE_REV,
> +};
> +
> +struct fd5121_data {
> + struct attribute_group *groups[3];
> + struct pmbus_driver_info info;
> + struct device *dev;
> + u32 id;
> +};
> +
> +static s32 fd5121_read_block_data(const struct i2c_client *client,
> + u8 cmd_code, u8 len, u8 *pbuf)
> +{
> + s32 ret = 0;
> +
> + if (!i2c_check_functionality(client->adapter,
> + I2C_FUNC_SMBUS_READ_BLOCK_DATA)) {
> +
> + /* Payload length is in the first byte. */
> + ret = i2c_smbus_read_i2c_block_data(client, cmd_code,
> + len, pbuf);
> + if (ret < 0)
> + return ret;
> + ret = pbuf[0];
> + if (ret > len)
> + ret = len;
> + for (int idx = 0; idx < ret; idx++)
> + pbuf[idx] = pbuf[idx + 1];
> + return ret;
> + }
> + ret = i2c_smbus_read_block_data(client, cmd_code, pbuf);
> + if (ret < 0)
> + return ret;
> + return min_t(s32, ret, len);
> +}
> +
> +/* Command code that expects block read, not word read */
> +static bool fd5121_blk_rd_reg(u8 cmd_code)
> +{
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(blk_rd_cc); i++) {
> + if (cmd_code == blk_rd_cc[i])
> + return true;
> + }
> + return false;
> +}
> +
> +static ssize_t fd5121_send_byte_store(struct device *dev,
> + struct device_attribute *da,
> + const char *buf, size_t count)
> +{
> + struct i2c_client *client = to_i2c_client(dev->parent);
> + u8 val = 0;
> + int ret;
> +
> + ret = kstrtou8(buf, 10, &val);
> + if (ret < 0)
> + return ret;
> + ret = i2c_smbus_write_byte(client, val);
> + if (ret < 0)
> + return ret;
> + return count;
> +}
> +
> +static int fd5121_config_activate(struct i2c_client *client)
> +{
> + return i2c_smbus_write_byte_data(client,
> + PMBUS_REG_MFR_CONFIG_ACTIVATE, 0xAA);
> +}
> +
> +static ssize_t fd5121_byte_store(struct device *dev,
> + struct device_attribute *da,
> + const char *buf, size_t count)
> +{
> + struct sensor_device_attribute *attr = to_sensor_dev_attr(da);
> + struct i2c_client *client = to_i2c_client(dev->parent);
> + u8 reg = attr->index;
> + int ret = 0;
> + u8 val = 0;
> +
> + switch (reg) {
> + case PMBUS_REG_MFR_CONFIG_ACTIVATE:
> + ret = fd5121_config_activate(client);
> + if (ret < 0)
> + return ret;
> + return count;
> + default:
> + ret = kstrtou8(buf, 10, &val);
> + if (ret < 0)
> + return ret;
> + break;
> + }
> + if (reg == PMBUS_PAGE && ((val != 0 && val != 1 &&
> + val != GENMASK(7, 0))))
> + return -EINVAL;
> + ret = i2c_smbus_write_byte_data(client, reg, val);
> + if (ret < 0)
> + return ret;
> + return count;
> +}
> +
> +static ssize_t fd5121_byte_show(struct device *dev,
> + struct device_attribute *da, char *buf)
> +{
> + struct sensor_device_attribute *attr = to_sensor_dev_attr(da);
> + struct i2c_client *client = to_i2c_client(dev->parent);
> + u8 reg = attr->index;
> + s32 ret;
> +
> + ret = i2c_smbus_read_byte_data(client, reg);
> + if (ret < 0)
> + return ret;
> + return scnprintf(buf, PAGE_SIZE, "%d\n", ret & 0xFF);
> +}
> +
> +static ssize_t fd5121_word_store(struct device *dev,
> + struct device_attribute *da,
> + const char *buf, size_t count)
> +{
> + struct sensor_device_attribute *attr = to_sensor_dev_attr(da);
> + struct i2c_client *client = to_i2c_client(dev->parent);
> + u8 reg = attr->index;
> + s16 val = 0;
> + int ret = 0;
> +
> + switch (reg) {
> + case PMBUS_REG_MFR_PWD_PROGRAM_RAM:
> + val = 0xC93F;
> + break;
> + default:
> + ret = kstrtos16(buf, 10, &val);
> + if (ret)
> + return ret;
> + break;
> + }
> + ret = i2c_smbus_write_word_data(client, reg, val);
> + if (ret < 0)
> + return ret;
> + return count;
> +}
> +
> +static ssize_t fd5121_word_show(struct device *dev,
> + struct device_attribute *da, char *buf)
> +{
> + struct sensor_device_attribute *attr = to_sensor_dev_attr(da);
> + struct i2c_client *client = to_i2c_client(dev->parent);
> + u8 data[I2C_SMBUS_BLOCK_MAX] = { 0 };
> + u8 reg = attr->index;
> + s32 ret = 0;
> +
> + if (fd5121_blk_rd_reg(reg)) {
> + ret = fd5121_read_block_data(client, reg, 2, data);
> + if (ret >= 0)
> + ret = get_unaligned_le16(data);
> + } else
> + ret = i2c_smbus_read_word_data(client, reg);
> + if (ret < 0)
> + return ret;
> + return scnprintf(buf, PAGE_SIZE, "%d\n", ret & 0xFFFF);
> +}
> +
> +static s32 fd5121_write_block_data(const struct i2c_client *client,
> + u8 cmd_code, u8 len, u8 *pbuf)
> +{
> + s32 ret = 0;
> +
> + if (!i2c_check_functionality(client->adapter,
> + I2C_FUNC_SMBUS_WRITE_BLOCK_DATA))
> + ret = i2c_smbus_write_i2c_block_data(client, cmd_code,
> + len, pbuf);
> + else
> + ret = i2c_smbus_write_block_data(client, cmd_code,
> + len, pbuf);
> + return ret;
> +}
> +
> +static s32 fd5121_read_long(struct i2c_client *client, u8 cmd_code, u32 *pval)
> +{
> + u8 buffer[I2C_SMBUS_BLOCK_MAX] = { 0 };
> + s32 ret;
> +
> + ret = fd5121_read_block_data(client, cmd_code, 4, buffer);
> + if (ret < 0)
> + return ret;
> + if (ret < 4)
> + return -EIO;
> +
> + *pval = get_unaligned_le32(buffer);
> + return 0;
> +}
> +
> +static ssize_t fd5121_long_store(struct device *dev,
> + struct device_attribute *da,
> + const char *buf, size_t count)
> +{
> + struct sensor_device_attribute *attr = to_sensor_dev_attr(da);
> + struct i2c_client *client = to_i2c_client(dev->parent);
> + u8 reg = attr->index;
> + u8 buffer[4];
> + u32 val = 0;
> + int ret = 0;
> + u8 len;
> +
> + ret = kstrtou32(buf, 10, &val);
> + if (ret)
> + return ret;
> +
> + len = (u8) sizeof(buffer);
> + for (u8 i = 0; i < len; i++) {
> + buffer[i] = val & 0xFF;
> + val >>= 8;
> + }
> + ret = fd5121_write_block_data(client, reg, len, buffer);
> + if (ret < 0)
> + return ret;
> + return count;
> +}
> +
> +static ssize_t fd5121_long_show(struct device *dev,
> + struct device_attribute *da, char *buf)
> +{
> + struct sensor_device_attribute *attr = to_sensor_dev_attr(da);
> + struct i2c_client *client = to_i2c_client(dev->parent);
> + u8 reg = attr->index;
> + u32 val = 0;
> + s32 ret = 0;
> +
> + ret = fd5121_read_long(client, reg, &val);
> + if (ret < 0)
> + return ret;
> + return scnprintf(buf, PAGE_SIZE, "%d\n", val);
> +}
> +
> +static ssize_t fd5121_block_show(struct device *dev,
> + struct device_attribute *da, char *buf)
> +{
> + struct i2c_client *client = to_i2c_client(dev->parent);
> + struct sensor_device_attribute *attr = to_sensor_dev_attr(da);
> + u8 buffer[I2C_SMBUS_BLOCK_MAX] = { 0 };
> + u8 reg = attr->index;
> + int printed = 0;
> + s32 ret = 0;
> + u8 len = 0;
> + int i = 0;
> +
> + if (reg == PMBUS_REG_MFR_FAULTY_SPS) {
> + int to_print = 0;
> +
> + len = 7;
> + ret = fd5121_read_block_data(client, reg, len, buffer);
> + if (ret < 0)
> + return ret;
> + printed = 0;
> + to_print = (ret < len) ? ret : len;
> + for (i = 0; i < to_print; i++)
> + printed += scnprintf(buf + printed,
> + PAGE_SIZE - printed,
> + "%02x", buffer[i]);
> + printed += scnprintf(buf + printed,
> + PAGE_SIZE - printed, "\n");
> + return printed;
> + } else if (reg == PMBUS_REG_BBR_RAM ||
> + reg == PMBUS_REG_BBR_OTP) {
> + u32 len = (reg == PMBUS_REG_BBR_OTP) ? 165 : 164;
> +
> + /* Extra byte may be needed in case we need to store
> + * the length of the data
> + */
> + u8 *tmp_in = kcalloc(len+1, sizeof(u8), GFP_KERNEL);
> +
> + if (tmp_in == NULL)
> + return -ENOMEM;
> + ret = fd5121_read_block_data(client, reg, len, tmp_in);
> + if (ret < 0) {
> + kfree(tmp_in);
> + return ret;
> + }
> +
> + printed = 0;
> + for (i = 0; i < ret; i++)
> + printed += scnprintf(buf + printed,
> + PAGE_SIZE - printed, "%02x",
> + tmp_in[i]);
> + printed += scnprintf(buf + printed,
> + PAGE_SIZE - printed, "\n");
> +
> + kfree(tmp_in);
> + return printed;
> + } else
> + return -ENODATA;
> +}
> +
> +static SENSOR_DEVICE_ATTR_RW(page, fd5121_byte,
> + PMBUS_PAGE);
> +static SENSOR_DEVICE_ATTR_RO(vout_raw, fd5121_word,
> + PMBUS_READ_VOUT);
> +static SENSOR_DEVICE_ATTR_RW(operation, fd5121_byte,
> + PMBUS_OPERATION);
> +static SENSOR_DEVICE_ATTR_RW(on_off_config, fd5121_byte,
> + PMBUS_ON_OFF_CONFIG);
> +static SENSOR_DEVICE_ATTR_WO(clear_faults, fd5121_byte,
> + PMBUS_CLEAR_FAULTS);
> +static SENSOR_DEVICE_ATTR_RW(write_protect, fd5121_byte,
> + PMBUS_WRITE_PROTECT);
> +static SENSOR_DEVICE_ATTR_RO(capability, fd5121_byte,
> + PMBUS_CAPABILITY);
> +static SENSOR_DEVICE_ATTR_RW(smbalert_mask, fd5121_word,
> + PMBUS_SMBALERT_MASK);
> +static SENSOR_DEVICE_ATTR_RO(vout_mode, fd5121_byte,
> + PMBUS_VOUT_MODE);
> +static SENSOR_DEVICE_ATTR_RW(vout_command, fd5121_word,
> + PMBUS_VOUT_COMMAND);
> +static SENSOR_DEVICE_ATTR_RW(vout_max, fd5121_word,
> + PMBUS_VOUT_MAX);
> +static SENSOR_DEVICE_ATTR_RW(vout_margin_high, fd5121_word,
> + PMBUS_VOUT_MARGIN_HIGH);
> +static SENSOR_DEVICE_ATTR_RW(vout_margin_low, fd5121_word,
> + PMBUS_VOUT_MARGIN_LOW);
> +static SENSOR_DEVICE_ATTR_RW(vout_transition_rate, fd5121_word,
> + PMBUS_VOUT_TRANSITION_RATE);
> +static SENSOR_DEVICE_ATTR_RW(vout_min, fd5121_word,
> + PMBUS_REG_VOUT_MIN);
> +static SENSOR_DEVICE_ATTR_RW(power_mode, fd5121_byte,
> + PMBUS_REG_POWER_MODE);
> +static SENSOR_DEVICE_ATTR_RW(vin_on, fd5121_word,
> + PMBUS_REG_VIN_ON);
> +static SENSOR_DEVICE_ATTR_RW(vin_off, fd5121_word,
> + PMBUS_REG_VIN_OFF);
> +static SENSOR_DEVICE_ATTR_RW(vin_uv_fault_response, fd5121_byte,
> + PMBUS_REG_VIN_UV_FAULT_RESPONSE);
> +static SENSOR_DEVICE_ATTR_RW(iin_oc_fault_response, fd5121_byte,
> + PMBUS_REG_IIN_OC_FAULT_RESPONSE);
> +static SENSOR_DEVICE_ATTR_RW(ton_delay, fd5121_word,
> + PMBUS_REG_TON_DELAY);
> +static SENSOR_DEVICE_ATTR_RW(pout_op_fault_response, fd5121_byte,
> + PMBUS_REG_POUT_OP_FAULT_RESPONSE);
> +static SENSOR_DEVICE_ATTR_RO(read_vaux, fd5121_word,
> + PMBUS_REG_READ_VAUX);
> +static SENSOR_DEVICE_ATTR_RW(iknee_set, fd5121_word,
> + PMBUS_REG_IKNEE_SET);
> +static SENSOR_DEVICE_ATTR_RW(pin_counter, fd5121_byte,
> + PMBUS_REG_PIN_COUNTER);
> +static SENSOR_DEVICE_ATTR_RW(vmin_aware, fd5121_word,
> + PMBUS_REG_VMIN_AWARE);
> +static SENSOR_DEVICE_ATTR_RW(vout_ov_fault_response, fd5121_byte,
> + PMBUS_VOUT_OV_FAULT_RESPONSE);
> +static SENSOR_DEVICE_ATTR_RW(vout_uv_fault_response, fd5121_byte,
> + PMBUS_VOUT_UV_FAULT_RESPONSE);
> +static SENSOR_DEVICE_ATTR_RW(iout_oc_fault_response, fd5121_byte,
> + PMBUS_IOUT_OC_FAULT_RESPONSE);
> +static SENSOR_DEVICE_ATTR_RW(ot_fault_response, fd5121_byte,
> + PMBUS_OT_FAULT_RESPONSE);
> +static SENSOR_DEVICE_ATTR_RW(vin_ov_fault_response, fd5121_byte,
> + PMBUS_VIN_OV_FAULT_RESPONSE);
> +static SENSOR_DEVICE_ATTR_RW(vaux_uv_fault_limit, fd5121_word,
> + PMBUS_REG_VAUX_UV_FAULT_LIMIT);
> +static SENSOR_DEVICE_ATTR_RW(vaux_ov_fault_limit, fd5121_word,
> + PMBUS_REG_VAUX_OV_FAULT_LIMIT);
> +static SENSOR_DEVICE_ATTR_RW(vaux_uv_fault_response, fd5121_byte,
> + PMBUS_REG_VAUX_UV_FAULT_RESPONSE);
> +static SENSOR_DEVICE_ATTR_RW(vaux_ov_fault_response, fd5121_byte,
> + PMBUS_REG_VAUX_OV_FAULT_RESPONSE);
> +static SENSOR_DEVICE_ATTR_RW(vaux_uv_warning, fd5121_word,
> + PMBUS_REG_VAUX_UV_WARNING);
> +static SENSOR_DEVICE_ATTR_RW(vaux_ov_warning, fd5121_word,
> + PMBUS_REG_VAUX_OV_WARNING);
> +static SENSOR_DEVICE_ATTR_RO(free_user_config_tables, fd5121_byte,
> + PMBUS_REG_MFR_FREE_USER_CONFIG_TABLES);
> +static SENSOR_DEVICE_ATTR_RW(address_table, fd5121_byte,
> + PMBUS_REG_MFR_ADDRESS_TABLE);
> +static SENSOR_DEVICE_ATTR_RW(status_onsemi, fd5121_word,
> + PMBUS_REG_MFR_STATUS_ONSEMI);
> +static SENSOR_DEVICE_ATTR_RO(status_byte, fd5121_byte,
> + PMBUS_STATUS_BYTE);
> +static SENSOR_DEVICE_ATTR_RO(status_cml, fd5121_byte,
> + PMBUS_STATUS_CML);
> +static SENSOR_DEVICE_ATTR_RO(status_other, fd5121_byte,
> + PMBUS_STATUS_OTHER);
> +static SENSOR_DEVICE_ATTR_RO(status_mfr_specific, fd5121_byte,
> + PMBUS_STATUS_MFR_SPECIFIC);
> +static SENSOR_DEVICE_ATTR_RO(revision, fd5121_byte,
> + PMBUS_REVISION);
> +static SENSOR_DEVICE_ATTR_RO(id, fd5121_long,
> + PMBUS_MFR_ID);
> +static SENSOR_DEVICE_ATTR_RO(model, fd5121_long,
> + PMBUS_MFR_MODEL);
> +static SENSOR_DEVICE_ATTR_RO(mfr_revision, fd5121_long,
> + PMBUS_MFR_REVISION);
> +static SENSOR_DEVICE_ATTR_RW(date, fd5121_word,
> + PMBUS_MFR_DATE);
> +static SENSOR_DEVICE_ATTR_RO(ic_device_id, fd5121_long,
> + PMBUS_IC_DEVICE_ID);
> +static SENSOR_DEVICE_ATTR_RO(ic_device_rev, fd5121_word,
> + PMBUS_IC_DEVICE_REV);
> +static SENSOR_DEVICE_ATTR_WO(unlock, fd5121_byte,
> + PMBUS_REG_MFR_UNLOCK);
> +static SENSOR_DEVICE_ATTR_RO(faulty_sps, fd5121_block,
> + PMBUS_REG_MFR_FAULTY_SPS);
> +static SENSOR_DEVICE_ATTR_RO(tlvr_faults, fd5121_word,
> + PMBUS_REG_TLVR_FAULTS);
> +static SENSOR_DEVICE_ATTR_RW(user_store_config_tab, fd5121_byte,
> + PMBUS_REG_MFR_USER_STORE_CONFIG_TAB);
> +static SENSOR_DEVICE_ATTR_RO(user_config_index, fd5121_byte,
> + PMBUS_REG_MFR_USER_CONFIG_INDEX);
> +static SENSOR_DEVICE_ATTR_RO(pwm_disconnection, fd5121_word,
> + PMBUS_REG_MFR_PWM_DISCONNECTION);
> +static SENSOR_DEVICE_ATTR_RO(vr_disconnection, fd5121_byte,
> + PMBUS_REG_MFR_VR_DISCONNECTION);
> +static SENSOR_DEVICE_ATTR_RW(ton_slew, fd5121_byte,
> + PMBUS_REG_MFR_TON_SLEW);
> +static SENSOR_DEVICE_ATTR_RW(toff_slew, fd5121_byte,
> + PMBUS_REG_MFR_TOFF_SLEW);
> +static SENSOR_DEVICE_ATTR_RW(rail_name, fd5121_word,
> + PMBUS_REG_MFR_RAIL_NAME);
> +static SENSOR_DEVICE_ATTR_RW(vout_droop, fd5121_byte,
> + PMBUS_REG_MFR_VOUT_DROOP);
> +static SENSOR_DEVICE_ATTR_WO(svr_go, fd5121_send_byte,
> + PMBUS_REG_MFR_SVR_GO);
> +static SENSOR_DEVICE_ATTR_RW(user_restore_config_tab, fd5121_byte,
> + PMBUS_REG_MFR_USER_RESTORE_CONFIG_TAB);
> +static SENSOR_DEVICE_ATTR_WO(set_pwd, fd5121_byte,
> + PMBUS_REG_MFR_SET_PWD);
> +static SENSOR_DEVICE_ATTR_RW(config_activate, fd5121_byte,
> + PMBUS_REG_MFR_CONFIG_ACTIVATE);
> +static SENSOR_DEVICE_ATTR_RW(config_recover, fd5121_byte,
> + PMBUS_REG_MFR_CONFIG_RECOVER);
> +static SENSOR_DEVICE_ATTR_RW(otp_dump, fd5121_byte,
> + PMBUS_REG_MFR_OTP_DUMP);
> +static SENSOR_DEVICE_ATTR_RW(bbr_en, fd5121_byte,
> + PMBUS_REG_MFR_BBR_EN);
> +static SENSOR_DEVICE_ATTR_RW(dpm_min, fd5121_byte,
> + PMBUS_REG_MFR_DPM_MIN);
> +static SENSOR_DEVICE_ATTR_RW(vboot, fd5121_word,
> + PMBUS_REG_MFR_VBOOT);
> +static SENSOR_DEVICE_ATTR_RW(preclamp_offset, fd5121_word,
> + PMBUS_REG_MFR_PRECLAMP_OFFSET);
> +static SENSOR_DEVICE_ATTR_RW(tlvr_diagn, fd5121_word,
> + PMBUS_REG_MFR_TLVR_DIAGN);
> +static SENSOR_DEVICE_ATTR_RO(vsys, fd5121_word,
> + PMBUS_REG_MFR_READ_VSYS);
> +static SENSOR_DEVICE_ATTR_RW(specific_e9, fd5121_word,
> + PMBUS_REG_MFR_SPECIFIC_E9);
> +static SENSOR_DEVICE_ATTR_RW(specific_ea, fd5121_long,
> + PMBUS_REG_MFR_SPECIFIC_EA);
> +static SENSOR_DEVICE_ATTR_RO(ss_cbc, fd5121_word,
> + PMBUS_REG_MFR_SS_CBC);
> +static SENSOR_DEVICE_ATTR_RO(amd_status, fd5121_byte,
> + PMBUS_REG_MFR_AMD_STATUS);
> +static SENSOR_DEVICE_ATTR_RO(checksum, fd5121_word,
> + PMBUS_REG_MFR_CHECKSUM);
> +static SENSOR_DEVICE_ATTR_RO(cse_index, fd5121_word,
> + PMBUS_REG_CSE_INDEX);
> +static SENSOR_DEVICE_ATTR_RW(cout_measure, fd5121_word,
> + PMBUS_REG_COUT_MEASURE);
> +static SENSOR_DEVICE_ATTR_RO(vr_cout, fd5121_word,
> + PMBUS_REG_VR_COUT);
> +static SENSOR_DEVICE_ATTR_RO(bbr_ram, fd5121_block,
> + PMBUS_REG_BBR_RAM);
> +static SENSOR_DEVICE_ATTR_RO(bbr_otp, fd5121_block,
> + PMBUS_REG_BBR_OTP);
> +static SENSOR_DEVICE_ATTR_RO(psys, fd5121_word,
> + PMBUS_REG_READ_PSYS);
> +static SENSOR_DEVICE_ATTR_RW(postclamp_offset, fd5121_word,
> + PMBUS_REG_POSTCLAMP_OFFSET);
> +static SENSOR_DEVICE_ATTR_RW(pgood_delay, fd5121_byte,
> + PMBUS_REG_PGOOD_DELAY);
> +static SENSOR_DEVICE_ATTR_RW(specific_f8, fd5121_word,
> + PMBUS_REG_MFR_SPECIFIC_F8);
> +static SENSOR_DEVICE_ATTR_RW(specific_f9, fd5121_long,
> + PMBUS_REG_MFR_SPECIFIC_F9);
> +static SENSOR_DEVICE_ATTR_RW(pwd_program_ram, fd5121_word,
> + PMBUS_REG_MFR_PWD_PROGRAM_RAM);
> +static SENSOR_DEVICE_ATTR_RW(pwd_program_i2c, fd5121_word,
> + PMBUS_REG_MFR_PWD_PROGRAM_I2C);
> +static SENSOR_DEVICE_ATTR_RW(pwd_enable_otp_store, fd5121_word,
> + PMBUS_REG_MFR_PWD_ENABLE_OTP_STORE);
> +
> +static struct attribute *fd5121_non_paged_attrs[] = {
> + &sensor_dev_attr_page.dev_attr.attr,
> + &sensor_dev_attr_capability.dev_attr.attr,
> + &sensor_dev_attr_pin_counter.dev_attr.attr,
> + &sensor_dev_attr_vaux_uv_fault_limit.dev_attr.attr,
> + &sensor_dev_attr_vaux_ov_fault_limit.dev_attr.attr,
> + &sensor_dev_attr_vaux_uv_warning.dev_attr.attr,
> + &sensor_dev_attr_vaux_ov_warning.dev_attr.attr,
> + &sensor_dev_attr_free_user_config_tables.dev_attr.attr,
> + &sensor_dev_attr_address_table.dev_attr.attr,
> + &sensor_dev_attr_unlock.dev_attr.attr,
> + &sensor_dev_attr_faulty_sps.dev_attr.attr,
> + &sensor_dev_attr_tlvr_faults.dev_attr.attr,
> + &sensor_dev_attr_user_store_config_tab.dev_attr.attr,
> + &sensor_dev_attr_user_config_index.dev_attr.attr,
> + &sensor_dev_attr_pwm_disconnection.dev_attr.attr,
> + &sensor_dev_attr_vr_disconnection.dev_attr.attr,
> + &sensor_dev_attr_user_restore_config_tab.dev_attr.attr,
> + &sensor_dev_attr_svr_go.dev_attr.attr,
> + &sensor_dev_attr_set_pwd.dev_attr.attr,
> + &sensor_dev_attr_config_activate.dev_attr.attr,
> + &sensor_dev_attr_config_recover.dev_attr.attr,
> + &sensor_dev_attr_otp_dump.dev_attr.attr,
> + &sensor_dev_attr_bbr_en.dev_attr.attr,
> + &sensor_dev_attr_vboot.dev_attr.attr,
> + &sensor_dev_attr_vsys.dev_attr.attr,
> + &sensor_dev_attr_specific_e9.dev_attr.attr,
> + &sensor_dev_attr_specific_ea.dev_attr.attr,
> + &sensor_dev_attr_ss_cbc.dev_attr.attr,
> + &sensor_dev_attr_checksum.dev_attr.attr,
> + &sensor_dev_attr_cse_index.dev_attr.attr,
> + &sensor_dev_attr_cout_measure.dev_attr.attr,
> + &sensor_dev_attr_vr_cout.dev_attr.attr,
> + &sensor_dev_attr_bbr_ram.dev_attr.attr,
> + &sensor_dev_attr_bbr_otp.dev_attr.attr,
> + &sensor_dev_attr_psys.dev_attr.attr,
> + &sensor_dev_attr_specific_f8.dev_attr.attr,
> + &sensor_dev_attr_specific_f9.dev_attr.attr,
> + &sensor_dev_attr_pwd_program_ram.dev_attr.attr,
> + &sensor_dev_attr_pwd_program_i2c.dev_attr.attr,
> + &sensor_dev_attr_pwd_enable_otp_store.dev_attr.attr,
> + &sensor_dev_attr_revision.dev_attr.attr,
> + &sensor_dev_attr_id.dev_attr.attr,
> + &sensor_dev_attr_model.dev_attr.attr,
> + &sensor_dev_attr_mfr_revision.dev_attr.attr,
> + &sensor_dev_attr_date.dev_attr.attr,
> + &sensor_dev_attr_ic_device_id.dev_attr.attr,
> + &sensor_dev_attr_ic_device_rev.dev_attr.attr,
> + NULL
> +};
> +
> +static struct attribute *fd5121_paged_attrs[] = {
> + &sensor_dev_attr_operation.dev_attr.attr,
> + &sensor_dev_attr_vout_raw.dev_attr.attr,
> + &sensor_dev_attr_on_off_config.dev_attr.attr,
> + &sensor_dev_attr_clear_faults.dev_attr.attr,
> + &sensor_dev_attr_write_protect.dev_attr.attr,
> + &sensor_dev_attr_smbalert_mask.dev_attr.attr,
> + &sensor_dev_attr_vout_mode.dev_attr.attr,
> + &sensor_dev_attr_vout_command.dev_attr.attr,
> + &sensor_dev_attr_vout_margin_high.dev_attr.attr,
> + &sensor_dev_attr_vout_margin_low.dev_attr.attr,
> + &sensor_dev_attr_vout_min.dev_attr.attr,
> + &sensor_dev_attr_vin_on.dev_attr.attr,
> + &sensor_dev_attr_vin_off.dev_attr.attr,
> + &sensor_dev_attr_vout_ov_fault_response.dev_attr.attr,
> + &sensor_dev_attr_vout_uv_fault_response.dev_attr.attr,
> + &sensor_dev_attr_iout_oc_fault_response.dev_attr.attr,
> + &sensor_dev_attr_ot_fault_response.dev_attr.attr,
> + &sensor_dev_attr_vin_ov_fault_response.dev_attr.attr,
> + &sensor_dev_attr_status_byte.dev_attr.attr,
> + &sensor_dev_attr_iknee_set.dev_attr.attr,
> + &sensor_dev_attr_vmin_aware.dev_attr.attr,
> + &sensor_dev_attr_power_mode.dev_attr.attr,
> + &sensor_dev_attr_vin_uv_fault_response.dev_attr.attr,
> + &sensor_dev_attr_iin_oc_fault_response.dev_attr.attr,
> + &sensor_dev_attr_ton_delay.dev_attr.attr,
> + &sensor_dev_attr_pout_op_fault_response.dev_attr.attr,
> + &sensor_dev_attr_vaux_uv_fault_response.dev_attr.attr,
> + &sensor_dev_attr_vaux_ov_fault_response.dev_attr.attr,
> + &sensor_dev_attr_status_onsemi.dev_attr.attr,
> + &sensor_dev_attr_status_cml.dev_attr.attr,
> + &sensor_dev_attr_status_other.dev_attr.attr,
> + &sensor_dev_attr_status_mfr_specific.dev_attr.attr,
> + &sensor_dev_attr_read_vaux.dev_attr.attr,
> + &sensor_dev_attr_ton_slew.dev_attr.attr,
> + &sensor_dev_attr_toff_slew.dev_attr.attr,
> + &sensor_dev_attr_rail_name.dev_attr.attr,
> + &sensor_dev_attr_vout_droop.dev_attr.attr,
> + &sensor_dev_attr_dpm_min.dev_attr.attr,
> + &sensor_dev_attr_preclamp_offset.dev_attr.attr,
> + &sensor_dev_attr_tlvr_diagn.dev_attr.attr,
> + &sensor_dev_attr_amd_status.dev_attr.attr,
> + &sensor_dev_attr_postclamp_offset.dev_attr.attr,
> + &sensor_dev_attr_pgood_delay.dev_attr.attr,
> + &sensor_dev_attr_vout_max.dev_attr.attr,
> + &sensor_dev_attr_vout_transition_rate.dev_attr.attr,
> + NULL
This is a complete no-go. We do not explose raw register data as sysfs
attributes. You may expose essential register data as debugfs files,
but only those deemed necessary. The above is just "let's blindly
expose everything". Most of the above should be programmed in manufacturing
and not be touched subsequently, much less as writeable attributes.
Writing bad/unexpected values into many of those attributes can turn
a board into a brick. It is bad enough that/if this is even possible,
but exposing it as sysfs attribute would be a terrible idea.
I am not going to review this driver any further at this point.
Guenter
> +};
> +
> +static struct attribute_group fd5121_groups[2] = {
> + { .name = "global", .attrs = fd5121_non_paged_attrs },
> + { .name = "paged", .attrs = fd5121_paged_attrs }
> +};
> +
> +/* Regulator descriptors for VOUT rails (VID encoded) */
> +static struct regulator_desc fd5121_reg_desc[] = {
> + PMBUS_REGULATOR_STEP_ONE("vout1", 3001, 1000, 200000),
> + PMBUS_REGULATOR_STEP_ONE("vout2", 3001, 1000, 200000),
> +};
> +
> +static int fd5121_valid_reg(struct i2c_client *client, int reg)
> +{
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(cc_list); i++) {
> + if (reg == cc_list[i])
> + return 0;
> + }
> +
> + if (fd5121_blk_rd_reg(reg))
> + return 0;
> + return -ENXIO;
> +}
> +
> +static int fd5121_read_word_data(struct i2c_client *client, int page,
> + int phase, int reg)
> +{
> + int ret;
> +
> + ret = fd5121_valid_reg(client, reg);
> + if (ret < 0)
> + return ret;
> +
> + ret = pmbus_read_word_data(client, page, phase, reg);
> + if (ret < 0)
> + return ret;
> +
> + /* Chip reports VOUT_MODE as vid. But gives raw value 1mV per bit.
> + * So, encode the READ_VOUT value so that it gets decoded and
> + * reported correctly.
> + */
> + if (reg == PMBUS_READ_VOUT)
> + ret = DIV_ROUND_CLOSEST(155000 - ret * 100, 625);
> + return ret;
> +}
> +
> +static int fd5121_read_byte_data(struct i2c_client *client, int page, int reg)
> +{
> + int ret;
> +
> + ret = fd5121_valid_reg(client, reg);
> + if (ret < 0)
> + return ret;
> +
> + return pmbus_read_byte_data(client, page, reg);
> +}
> +
> +static int fd5121_write_byte_data(struct i2c_client *client, int page,
> + int reg, u8 value)
> +{
> + int ret;
> +
> + ret = fd5121_valid_reg(client, reg);
> + if (ret < 0)
> + return ret;
> + return pmbus_write_byte_data(client, page, reg, value);
> +}
> +
> +static int fd5121_write_byte(struct i2c_client *client, int page, u8 byte)
> +{
> + return pmbus_write_byte(client, page, byte);
> +}
> +
> +static int fd5121_write_word_data(struct i2c_client *client, int page,
> + int reg, u16 word)
> +{
> + int ret;
> +
> + ret = fd5121_valid_reg(client, reg);
> + if (ret < 0)
> + return ret;
> + ret = pmbus_write_word_data(client, page, reg, word);
> + return ret;
> +}
> +
> +static u32 fd5121_get_dev_id(struct i2c_client *client)
> +{
> + u32 dev_id = 0;
> + s32 ret = 0;
> +
> + ret = fd5121_read_long(client, PMBUS_IC_DEVICE_ID, &dev_id);
> + if (ret < 0)
> + return CTLR_ID_UNKNOWN;
> +
> + switch (dev_id) {
> + case CTLR_ID_FD5121:
> + case CTLR_ID_FD5123:
> + case CTLR_ID_FD5125:
> + break;
> + default:
> + if (dev_id != 0)
> + dev_err(&client->dev, "Unknown device 0x%x",
> + dev_id);
> + return CTLR_ID_UNKNOWN;
> + }
> + return dev_id;
> +}
> +
> +static int fd5121_probe(struct i2c_client *client)
> +{
> + struct pmbus_driver_info *info;
> + struct fd5121_data *pdata;
> + u32 id;
> +
> + if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C))
> + return -EOPNOTSUPP;
> +
> + pdata = devm_kzalloc(&client->dev, sizeof(struct fd5121_data),
> + GFP_KERNEL);
> + if (pdata == NULL)
> + return -ENOMEM;
> +
> + pdata->dev = &client->dev;
> + pdata->groups[0] = &fd5121_groups[0];
> + pdata->groups[1] = &fd5121_groups[1];
> +
> + id = fd5121_get_dev_id(client);
> + if (id == CTLR_ID_UNKNOWN)
> + return -ENODEV;
> +
> + pdata->id = id;
> +
> + switch (id) {
> + case CTLR_ID_FD5121:
> + case CTLR_ID_FD5123:
> + case CTLR_ID_FD5125:
> + break;
> + default:
> + dev_err(&client->dev, "Failed to read device ID");
> + return -ENODEV;
> + }
> +
> + info = &pdata->info;
> + info->groups = (const struct attribute_group **)&pdata->groups[0];
> + info->write_word_data = fd5121_write_word_data;
> + info->write_byte = fd5121_write_byte;
> + info->write_byte_data = fd5121_write_byte_data;
> + info->read_word_data = fd5121_read_word_data;
> + info->read_byte_data = fd5121_read_byte_data;
> +
> + info->pages = FD5121_NUM_PAGES;
> + info->format[PSC_VOLTAGE_IN] = linear;
> + info->format[PSC_VOLTAGE_OUT] = vid;
> +
> + fd5121_reg_desc[0].id = 0;
> + fd5121_reg_desc[1].id = 1;
> +
> + /* Device implements VID coding with 1 mV steps from 0.200 V
> + * up to 3.200 V
> + */
> + info->num_regulators = FD5121_NUM_PAGES;
> + info->reg_desc = fd5121_reg_desc;
> + info->format[PSC_CURRENT_IN] = linear;
> + info->format[PSC_CURRENT_OUT] = linear;
> + info->format[PSC_POWER] = linear;
> + info->format[PSC_TEMPERATURE] = linear;
> + for (u8 idx = 0; idx < info->pages; idx++) {
> + info->func[idx] = PMBUS_HAVE_IOUT | PMBUS_HAVE_STATUS_IOUT;
> + info->func[idx] |= PMBUS_HAVE_VOUT | PMBUS_HAVE_STATUS_VOUT;
> + info->func[idx] |= PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP;
> + info->func[idx] |= PMBUS_HAVE_PIN | PMBUS_HAVE_POUT;
> + info->func[idx] |= PMBUS_HAVE_VIN | PMBUS_HAVE_IIN;
> + info->func[idx] |= PMBUS_HAVE_STATUS_INPUT;
> + info->vrm_version[idx] = amd625mv;
> + }
> + return pmbus_do_probe(client, info);
> +}
> +
> +#ifdef CONFIG_OF
> +static const struct of_device_id fd5121_of_match[] = {
> + { .compatible = "onnn,fd5121" },
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, fd5121_of_match);
> +#endif
> +
> +static const struct i2c_device_id fd5121_id[] = {
> + { "fd5121", chip_fd5121 },
> + { "fd5123", chip_fd5123 },
> + { "fd5125", chip_fd5125 },
> + { }
> +};
> +MODULE_DEVICE_TABLE(i2c, fd5121_id);
> +
> +static struct i2c_driver fd5121_driver = {
> + .driver = {
> + .name = "fd5121",
> + .of_match_table = of_match_ptr(fd5121_of_match),
> + },
> + .probe = fd5121_probe,
> + .id_table = fd5121_id,
> +};
> +
> +module_i2c_driver(fd5121_driver);
> +
> +MODULE_AUTHOR("Selva Rajagopal <selvamani.rajagopal@onsemi.com>");
> +MODULE_DESCRIPTION("PMBus driver for FD5121");
> +MODULE_LICENSE("GPL");
> +MODULE_IMPORT_NS("PMBUS");
> +
>
^ permalink raw reply
* Re: [PATCH RFC 5/8] clk: sunxi-ng: a733: Add bus clocks support
From: Andre Przywara @ 2026-06-23 22:35 UTC (permalink / raw)
To: Junhui Liu, Michael Turquette, Stephen Boyd, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Chen-Yu Tsai, Jernej Skrabec,
Samuel Holland, Philipp Zabel, Paul Walmsley, Palmer Dabbelt,
Albert Ou, Alexandre Ghiti, Richard Cochran
Cc: linux-clk, devicetree, linux-arm-kernel, linux-sunxi,
linux-kernel, linux-riscv, netdev
In-Reply-To: <20260310-a733-clk-v1-5-36b4e9b24457@pigmoral.tech>
Hi,
On 3/10/26 08:33, Junhui Liu wrote:
> Add the essential bus clocks in the Allwinner A733 CCU, including AHB,
> APB0, APB1, APB_UART, NSI, and MBUS. These buses are necessary for many
> other functional modules. Additionally clocks such as trace, gic and
> cpu_peri are also added as they fall within the register address range
> of the bus clocks, even though they are not strictly bus clocks.
>
> The MBUS clock is marked as critical to ensure the memory bus remains
> operational at all times. For the NSI and MBUS clocks, the hardware
> requires an update bit (bit 27) to be set so that the configuration
> takes effect and the updated parameters can be correctly read back.
>
> Signed-off-by: Junhui Liu <junhui.liu@pigmoral.tech>
> ---
> drivers/clk/sunxi-ng/ccu-sun60i-a733.c | 131 +++++++++++++++++++++++++++++++++
> 1 file changed, 131 insertions(+)
>
> diff --git a/drivers/clk/sunxi-ng/ccu-sun60i-a733.c b/drivers/clk/sunxi-ng/ccu-sun60i-a733.c
> index cf819504c51f..68457813dbbb 100644
> --- a/drivers/clk/sunxi-ng/ccu-sun60i-a733.c
> +++ b/drivers/clk/sunxi-ng/ccu-sun60i-a733.c
> @@ -19,6 +19,7 @@
> #include "ccu_common.h"
>
> #include "ccu_div.h"
> +#include "ccu_mp.h"
> #include "ccu_mult.h"
> #include "ccu_nkmp.h"
> #include "ccu_nm.h"
> @@ -65,6 +66,16 @@ static const struct clk_hw *pll_ref_hws[] = {
> &pll_ref_clk.common.hw
> };
>
> +/*
> + * There is a non-software-configurable mux selecting between the DCXO and the
> + * PLL_REF in hardware, whose output is fed to the sys-24M clock. Although both
> + * sys-24M and pll-ref are fixed at 24 MHz, define a 1:1 fixed factor clock to
> + * provide logical separation:
> + * - pll-ref is dedicated to feeding other PLLs
> + * - sys-24M serves as reference clock for downstream functional modules
> + */
> +static CLK_FIXED_FACTOR_HWS(sys_24M_clk, "sys-24M", pll_ref_hws, 1, 1, 0);
> +
> #define SUN60I_A733_PLL_DDR_REG 0x020
> static struct ccu_nkmp pll_ddr_clk = {
> .enable = BIT(27),
> @@ -371,6 +382,107 @@ static SUNXI_CCU_M_HWS(pll_de_4x_clk, "pll-de-4x", pll_de_hws,
> static SUNXI_CCU_M_HWS(pll_de_3x_clk, "pll-de-3x", pll_de_hws,
> SUN60I_A733_PLL_DE_REG, 16, 3, 0);
>
> +/**************************************************************************
> + * bus clocks *
> + **************************************************************************/
> +
> +static const struct clk_parent_data ahb_apb_parents[] = {
> + { .hw = &sys_24M_clk.hw },
> + { .fw_name = "losc" },
> + { .fw_name = "iosc" },
> + { .hw = &pll_periph0_600M_clk.hw },
> +};
> +
> +static SUNXI_CCU_M_DATA_WITH_MUX(ahb_clk, "ahb", ahb_apb_parents, 0x500,
> + 0, 5, /* M */
> + 24, 2, /* mux */
> + 0);
> +
> +static SUNXI_CCU_M_DATA_WITH_MUX(apb0_clk, "apb0", ahb_apb_parents, 0x510,
> + 0, 5, /* M */
> + 24, 2, /* mux */
> + 0);
> +
> +static SUNXI_CCU_M_DATA_WITH_MUX(apb1_clk, "apb1", ahb_apb_parents, 0x518,
> + 0, 5, /* M */
> + 24, 2, /* mux */
> + 0);
> +
> +static const struct clk_parent_data apb_uart_parents[] = {
> + { .hw = &sys_24M_clk.hw },
> + { .fw_name = "losc" },
> + { .fw_name = "iosc" },
> + { .hw = &pll_periph0_600M_clk.hw },
> + { .hw = &pll_periph0_480M_clk.common.hw },
> +};
> +static SUNXI_CCU_M_DATA_WITH_MUX(apb_uart_clk, "apb-uart", apb_uart_parents, 0x538,
> + 0, 5, /* M */
> + 24, 3, /* mux */
> + 0);
> +
> +static const struct clk_parent_data trace_parents[] = {
> + { .hw = &sys_24M_clk.hw },
> + { .fw_name = "losc" },
> + { .fw_name = "iosc" },
> + { .hw = &pll_periph0_300M_clk.hw },
> + { .hw = &pll_periph0_400M_clk.hw },
> +};
> +static SUNXI_CCU_M_DATA_WITH_MUX_GATE(trace_clk, "trace", trace_parents, 0x540,
> + 0, 5, /* M */
> + 24, 3, /* mux */
> + BIT(31), /* gate */
> + 0);
> +
> +static const struct clk_parent_data gic_cpu_peri_parents[] = {
> + { .hw = &sys_24M_clk.hw },
> + { .fw_name = "losc" },
> + { .hw = &pll_periph0_600M_clk.hw },
> + { .hw = &pll_periph0_480M_clk.common.hw },
> + { .hw = &pll_periph0_400M_clk.hw },
> +};
> +static SUNXI_CCU_M_DATA_WITH_MUX_GATE(gic_clk, "gic", gic_cpu_peri_parents, 0x560,
Do we really want to model the GIC clock? The A523 has one as well, as
we don't describe it there. And while the GICv3 binding describes a
clock property, the Linux driver completely ignores that.
So if I see this correctly, this clock would become unused, and would be
turned off, killing the GIC? So we would at least need a CLK_IS_CRITICAL
flag?
But it's a good reminder to lift this clock to something PLL based, in
U-Boot's SPL, because I guess the 24MHz are rather slow.
> + 0, 5, /* M */
> + 24, 3, /* mux */
> + BIT(31), /* gate */
> + 0);
> +
> +static SUNXI_CCU_M_DATA_WITH_MUX_GATE(cpu_peri_clk, "cpu-peri", gic_cpu_peri_parents, 0x568,
What is this clock about? I don't see it referenced by any peripheral in
the manual.
> + 0, 5, /* M */
> + 24, 3, /* mux */
> + BIT(31), /* gate */
> + 0);
> +
> +static const struct clk_parent_data nsi_parents[] = {
> + { .hw = &sys_24M_clk.hw },
> + { .hw = &pll_ddr_clk.common.hw },
> + { .hw = &pll_periph0_800M_clk.common.hw },
> + { .hw = &pll_periph0_600M_clk.hw },
> + { .hw = &pll_periph0_480M_clk.common.hw },
> + { .hw = &pll_de_3x_clk.common.hw },
> +};
> +static SUNXI_CCU_MP_DATA_WITH_MUX_GATE_FEAT(nsi_clk, "nsi", nsi_parents, 0x580,
Similar question like for the GIC: do we need this in the kernel, and do
we need to prevent this from being turned off?
> + 0, 5, /* M */
> + 0, 0, /* no P */
> + 24, 3, /* mux */
> + BIT(31), /* gate */
> + 0, CCU_FEATURE_UPDATE_BIT);
> +
> +static const struct clk_parent_data mbus_parents[] = {
> + { .hw = &sys_24M_clk.hw },
> + { .hw = &pll_periph1_600M_clk.hw },
> + { .hw = &pll_ddr_clk.common.hw },
> + { .hw = &pll_periph1_480M_clk.common.hw },
> + { .hw = &pll_periph1_400M_clk.hw },
> + { .hw = &pll_npu_clk.common.hw },
> +};
> +static SUNXI_CCU_MP_DATA_WITH_MUX_GATE_FEAT(mbus_clk, "mbus", mbus_parents, 0x588,
> + 0, 5, /* M */
> + 0, 0, /* no P */
> + 24, 3, /* mux */
> + BIT(31), /* gate */
> + CLK_IS_CRITICAL,
> + CCU_FEATURE_UPDATE_BIT);
> +
> /*
> * Contains all clocks that are controlled by a hardware register. They
> * have a (sunxi) .common member, which needs to be initialised by the common
> @@ -407,11 +519,21 @@ static struct ccu_common *sun60i_a733_ccu_clks[] = {
> &pll_de_clk.common,
> &pll_de_4x_clk.common,
> &pll_de_3x_clk.common,
> + &ahb_clk.common,
> + &apb0_clk.common,
> + &apb1_clk.common,
> + &apb_uart_clk.common,
> + &trace_clk.common,
> + &gic_clk.common,
> + &cpu_peri_clk.common,
> + &nsi_clk.common,
> + &mbus_clk.common,
> };
>
> static struct clk_hw_onecell_data sun60i_a733_hw_clks = {
> .hws = {
> [CLK_PLL_REF] = &pll_ref_clk.common.hw,
> + [CLK_SYS_24M] = &sys_24M_clk.hw,
> [CLK_PLL_DDR] = &pll_ddr_clk.common.hw,
> [CLK_PLL_PERIPH0_4X] = &pll_periph0_4x_clk.common.hw,
> [CLK_PLL_PERIPH0_2X] = &pll_periph0_2x_clk.common.hw,
> @@ -453,6 +575,15 @@ static struct clk_hw_onecell_data sun60i_a733_hw_clks = {
> [CLK_PLL_DE] = &pll_de_clk.common.hw,
> [CLK_PLL_DE_4X] = &pll_de_4x_clk.common.hw,
> [CLK_PLL_DE_3X] = &pll_de_3x_clk.common.hw,
> + [CLK_AHB] = &ahb_clk.common.hw,
> + [CLK_APB0] = &apb0_clk.common.hw,
> + [CLK_APB1] = &apb1_clk.common.hw,
> + [CLK_APB_UART] = &apb_uart_clk.common.hw,
> + [CLK_TRACE] = &trace_clk.common.hw,
> + [CLK_GIC] = &gic_clk.common.hw,
> + [CLK_CPU_PERI] = &cpu_peri_clk.common.hw,
> + [CLK_NSI] = &nsi_clk.common.hw,
> + [CLK_MBUS] = &mbus_clk.common.hw,
> },
> .num = CLK_FANOUT3 + 1,
> };
>
^ permalink raw reply
* Re: [PATCH v3 1/2] dt-bindings: PCI: qcom: Document the Hawi PCIe Controller
From: Matthew Leung @ 2026-06-23 22:47 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Bjorn Helgaas, Lorenzo Pieralisi, Krzysztof Wilczyński,
Manivannan Sadhasivam, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Bjorn Andersson, linux-arm-msm, linux-pci,
devicetree, linux-kernel
In-Reply-To: <20260622-brainy-observant-crocodile-d57bef@quoll>
On Mon, Jun 22, 2026 at 12:12:04PM +0200, Krzysztof Kozlowski wrote:
> On Thu, Jun 18, 2026 at 10:00:32PM +0000, Matthew Leung wrote:
> > Add a dedicated schema for the PCIe controllers found on the Hawi
> > platform.
> >
> > Signed-off-by: Matthew Leung <matthew.leung@oss.qualcomm.com>
> > ---
> > .../devicetree/bindings/pci/qcom,hawi-pcie.yaml | 202 +++++++++++++++++++++
> > 1 file changed, 202 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/pci/qcom,hawi-pcie.yaml b/Documentation/devicetree/bindings/pci/qcom,hawi-pcie.yaml
> > new file mode 100644
> > index 000000000000..fb3145f89f7f
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/pci/qcom,hawi-pcie.yaml
> > @@ -0,0 +1,202 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/pci/qcom,hawi-pcie.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Qualcomm Hawi PCI Express Root Complex
> > +
> > +maintainers:
> > + - Bjorn Andersson <andersson@kernel.org>
> > + - Manivannan Sadhasivam <mani@kernel.org>
> > +
> > +description:
> > + Qualcomm Hawi SoC (and compatible) PCIe root complex controller is based on
> > + the Synopsys DesignWare PCIe IP.
> > +
> > +properties:
> > + compatible:
> > + const: qcom,hawi-pcie
> > +
> > + reg:
> > + minItems: 5
> > + items:
> > + - description: Qualcomm specific registers
> > + - description: DesignWare PCIe registers
> > + - description: External local bus interface registers
> > + - description: ATU address space
> > + - description: PCIe configuration space
> > + - description: MHI registers
>
> Why is MHI optional?
The optional properties were carried over from the previous binding
which this binding was derived from, but I see those need not be the
case here. Each of the properties are used in both instances.
I will update with making the optional properties required.
>
> > +
> > + reg-names:
> > + minItems: 5
> > + items:
> > + - const: parf
> > + - const: dbi
> > + - const: elbi
> > + - const: atu
> > + - const: config
> > + - const: mhi
> > +
> > + clocks:
> > + minItems: 6
> > + items:
> > + - description: PCIe Auxiliary clock
> > + - description: PCIe Configuration clock
> > + - description: PCIe Master AXI clock
> > + - description: PCIe Slave AXI clock
> > + - description: PCIe Slave Q2A AXI clock
> > + - description: PCIe Aggre NoC AXI clock
> > + - description: PCIe Config NoC AXI clock
>
> Same here - does that mean that once instance does not have this clock?
> If so, mention this in commit msg.
Same; will make required.
>
> > +
> > + clock-names:
> > + minItems: 6
> > + items:
> > + - const: aux
> > + - const: cfg
> > + - const: bus_master
> > + - const: bus_slave
> > + - const: slave_q2a
> > + - const: noc_aggr
> > + - const: cnoc_sf_axi
> > +
> > + interrupts:
> > + minItems: 8
> > + maxItems: 9
> > +
> > + interrupt-names:
> > + minItems: 8
> > + items:
> > + - const: msi0
> > + - const: msi1
> > + - const: msi2
> > + - const: msi3
> > + - const: msi4
> > + - const: msi5
> > + - const: msi6
> > + - const: msi7
> > + - const: global
>
> Here as well - why is global optional?
Same; will make required.
>
> > +
> > + resets:
> > + minItems: 1
>
> Same here?
Same; will make required.
>
> > + items:
> > + - description: PCIe core reset
> > + - description: PCIe link down reset
> > +
> > + reset-names:
> > + minItems: 1
> > + items:
> > + - const: pci
> > + - const: link_down
> > +
> > +required:
> > + - power-domains
> > + - resets
> > + - reset-names
> > +
> > +allOf:
> > + - $ref: qcom,pcie-common.yaml#
>
> Best regards,
> Krzysztof
>
^ permalink raw reply
* Re: [PATCH v3 1/3] spi: dt-bindings: snps,dw-apb-ssi: add 'power-domains' property
From: Wolfram Sang @ 2026-06-23 23:07 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: linux-renesas-soc, Herve Codina, Mark Brown, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, linux-spi, devicetree
In-Reply-To: <20260623-banana-bettong-from-saturn-417cce@quoll>
[-- Attachment #1: Type: text/plain, Size: 385 bytes --]
> > + power-domains:
> > + maxItems: 1
>
> You should explain in the commit msg that likely other devices belong to
> power domain as well or likely can belong. Otherwise based on the commit
> msg I would suggest restricting it per variant... except that indeed
> these other variants could have a power domain.
Okay, if 'likely' is enough for you, I can reword it.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* RE: [PATCH 3/3] hwmon: (pmbus/fd5121): Add support FD5121, FD5123 and FD5125
From: Selvamani Rajagopal @ 2026-06-23 23:12 UTC (permalink / raw)
To: Guenter Roeck, Jonathan Corbet, Shuah Khan, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: linux-hwmon@vger.kernel.org, linux-doc@vger.kernel.org,
linux-kernel@vger.kernel.org, devicetree@vger.kernel.org
In-Reply-To: <320c44c9-924f-4b7e-a46a-37a72fa7267f@roeck-us.net>
> -----Original Message-----
> From: Guenter Roeck <groeck7@gmail.com> On Behalf Of Guenter Roeck
> Subject: Re: [PATCH 3/3] hwmon: (pmbus/fd5121): Add support FD5121, FD5123 and
> FD5125
>
>
> > + &sensor_dev_attr_vout_transition_rate.dev_attr.attr,
> > + NULL
>
> This is a complete no-go. We do not explose raw register data as sysfs
> attributes. You may expose essential register data as debugfs files,
> but only those deemed necessary. The above is just "let's blindly
> expose everything". Most of the above should be programmed in manufacturing
> and not be touched subsequently, much less as writeable attributes.
> Writing bad/unexpected values into many of those attributes can turn
> a board into a brick. It is bad enough that/if this is even possible,
> but exposing it as sysfs attribute would be a terrible idea.
>
> I am not going to review this driver any further at this point.
>
> Guenter
>
Thanks for your comment. Understood and agree on comment on exposing everything without enforcing what is being written/read.
We will discuss internally to see how to go about it. We need some custom registers for customers. We will identify those and
expose under debugfs, if needed.
> >
^ permalink raw reply
* Re: [PATCH 3/3] net: ipa: Add IPA v5.1 data
From: sashiko-bot @ 2026-06-24 1:44 UTC (permalink / raw)
To: Esteban Urrutia via B4 Relay; +Cc: conor+dt, devicetree, robh
In-Reply-To: <20260622-sm8450-ipa-v1-3-532f0299f96e@proton.me>
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Driver probe failure on SM8450 due to intentional SMEM size mismatch.
- [High] Overlapping IPA local memory regions will cause silent data corruption between filter tables and routing statistics.
- [Medium] The AP_MODEM_RX endpoint is incorrectly assigned to the DST_UL resource group, causing resource starvation and unused DST_DL resources.
--
commit 6da897f459f64d1618e077cb44111c9397ad478b
Author: Esteban Urrutia <esteuwu@proton.me>
net: ipa: Add IPA v5.1 data
Add the required ipa_data-v5.1.c file for IPA v5.1 along with changes
that declare IPA v5.1 support. This version of IPA is used in both SM8450
and SM8475 SoCs.
> diff --git a/drivers/net/ipa/data/ipa_data-v5.1.c b/drivers/net/ipa/data/ipa_data-v5.1.c
> new file mode 100644
> index 0000000000000..85b21efa1224d
> --- /dev/null
> +++ b/drivers/net/ipa/data/ipa_data-v5.1.c
[ ... ]
> + [IPA_ENDPOINT_AP_MODEM_RX] = {
> + .ee_id = GSI_EE_AP,
> + .channel_id = 1,
> + .endpoint_id = 23,
> + .toward_ipa = false,
> + .channel = {
> + .tre_count = 256,
> + .event_count = 256,
> + .tlv_count = 9,
> + },
> + .endpoint = {
> + .config = {
> + .resource_group = IPA_RSRC_GROUP_DST_UL,
[Severity: Medium]
Will assigning this downlink endpoint to the UL resource group cause
resource starvation?
Since this is the AP_MODEM_RX endpoint, should it be assigned to
IPA_RSRC_GROUP_DST_DL instead?
Later in the ipa_resource_dst[] configuration, 5 data sectors are allocated
to IPA_RSRC_GROUP_DST_DL. If no endpoints are assigned to the DL group,
those sectors will remain unused while AP_MODEM_RX competes with AP_LAN_RX
for the limited DST_UL resources.
[ ... ]
> +/* IPA-resident memory region data for an SoC having IPA v5.1 */
> +static const struct ipa_mem ipa_mem_local_data[] = {
[ ... ]
> + {
> + .id = IPA_MEM_STATS_TETHERING,
> + .offset = 0x2910,
> + .size = 0x03c0,
> + .canary_count = 0,
> + },
> + {
> + .id = IPA_MEM_AP_V4_FILTER,
> + .offset = 0x29b8,
> + .size = 0x0188,
> + .canary_count = 2,
> + },
> + {
> + .id = IPA_MEM_AP_V6_FILTER,
> + .offset = 0x2b40,
> + .size = 0x0228,
> + .canary_count = 0,
> + },
> + {
> + .id = IPA_MEM_STATS_FILTER_ROUTE,
> + .offset = 0x2cd0,
> + .size = 0x0ba0,
> + .canary_count = 2,
> + },
[Severity: High]
Do these memory region offsets overlap and lead to data corruption?
IPA_MEM_STATS_TETHERING starts at 0x2910 and has a size of 0x03c0, which
means it extends up to 0x2cd0.
However, IPA_MEM_AP_V4_FILTER begins at 0x29b8, placing it entirely inside
the tethering region. Similarly, IPA_MEM_AP_V6_FILTER starts at 0x2b40,
which overlaps both the end of the tethering region and the start of
IPA_MEM_STATS_FILTER_ROUTE.
Could hardware updates to the routing and filter stats corrupt the AP
filter tables here?
[ ... ]
> +/* Memory configuration data for an SoC having IPA v5.1 */
> +static const struct ipa_mem_data ipa_mem_data = {
> + .local_count = ARRAY_SIZE(ipa_mem_local_data),
> + .local = ipa_mem_local_data,
> + .imem_addr = 0x146a8000,
> + .imem_size = 0x00002000,
> + /*
> + * While this value is 0xb000 on SM8450 and 0x9000 on SM8475,
> + * it has been left set to 0x9000 for compatibility with SM8475
> + */
> + .smem_size = 0x00009000,
[Severity: High]
Will this hardcoded SMEM size cause the driver probe to fail on SM8450?
When the SM8450 bootloader or modem pre-allocates 0xb000 bytes,
qcom_smem_get() will return that actual size.
Looking at ipa_smem_init() in drivers/net/ipa/ipa_mem.c:
if (ret && actual != size) {
dev_err(dev, "SMEM item has size %zu, expected %zu\n",
actual, size);
return -EINVAL;
}
Because this strictly validates the returned size against
ipa->mem_data->smem_size, setting this to 0x9000 for SM8475 compatibility
would cause the size check to fail and abort the driver probe.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260622-sm8450-ipa-v1-0-532f0299f96e@proton.me?part=3
^ permalink raw reply
* Re: [PATCH 1/3] arm64: dts: qcom: sm8450: Add IPA support
From: Esteban Urrutia @ 2026-06-24 1:52 UTC (permalink / raw)
To: Konrad Dybcio, Bjorn Andersson, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Andrew Lunn, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Alex Elder
Cc: linux-arm-msm, devicetree, linux-kernel, netdev
In-Reply-To: <806046b2-20ed-437e-a7e6-b3c0699f5a2d@oss.qualcomm.com>
On 6/23/26 5:37 AM, Konrad Dybcio wrote:
> size = 0xb0000 for the RAM and uC regions that the driver seems
> to poke at (at a glance anyway..)
Sorry, I don't quite understand. Could you please clarify?
> base=0x1468_0000
> size=0x40_000
Noted, will fix in v2.
Regards,
Esteban
^ permalink raw reply
* Re: [PATCH 0/3] SM8450 IPA support
From: Esteban Urrutia @ 2026-06-24 1:57 UTC (permalink / raw)
To: Alex Elder, Bjorn Andersson, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Andrew Lunn, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Alex Elder
Cc: linux-arm-msm, devicetree, linux-kernel, netdev
In-Reply-To: <959db395-ae71-4a50-bd46-ac5add545a52@riscstar.com>
On 6/23/26 11:56 AM, Alex Elder wrote:
> I assume you have implemented this based on what you found in
> some downstream code. And if so, could you please indicate
> where to find that (so I can do some cross-referencing myself).
> I no longer have access to any Qualcomm internal documentation.
Hello. Yes, that would be the case. What I used goes as follows.
1. My personal findings regarding IPA:
https://gist.github.com/esteuwu/bd49ed67ed9290f41612bdae1cacb5bc
Note that these may be subject to errors since I mostly cross-checked
values to get here.
2. SM8450 downstream device tree:
https://github.com/LineageOS/android_kernel_qcom_sm8450-devicetrees/blob/lineage-20/qcom/waipio.dtsi#L3304
3. SM8475 downstream device tree:
https://github.com/LineageOS/android_kernel_qcom_sm8450-devicetrees/blob/lineage-20/qcom/cape.dtsi#L2624
It's worth mentioning that between SM8450 and SM8475, IPA SRAM size is
different, so I used the smaller SRAM size to support SM8475 as well. Hence
the reason why I included SM8475's downstream device tree as well.
4. SM8450/SM8475 downstream IPA driver:
https://github.com/LineageOS/android_kernel_qcom_sm8450-modules/tree/lineage-20/qcom/opensource/dataipa
Most of my cross-checking came from the source code in this folder.
Finally, for some values such as qmap, aggregation, tre_count and
event_count, I had to cross-check on the same folder that all
ipa_data-vX.Y.c files reside, since I couldn't find any reference to these
values in downstream code.
Regards,
Esteban
^ permalink raw reply
* Re: [PATCH 4/6] clk: qcom: Add Nord display clock controller support
From: Shawn Guo @ 2026-06-24 2:42 UTC (permalink / raw)
To: Taniya Das
Cc: Bjorn Andersson, Michael Turquette, Stephen Boyd, Brian Masney,
Bartosz Golaszewski, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Neil Armstrong, Konrad Dybcio, Ajit Pandey,
Imran Shaik, Jagadeesh Kona, linux-arm-msm, linux-clk,
linux-kernel, devicetree
In-Reply-To: <20260623-nords_mm_v1-v1-4-860c84539804@oss.qualcomm.com>
On Tue, Jun 23, 2026 at 04:24:07PM +0530, Taniya Das wrote:
> Add support for the display clock controllers (DISPCC) on the
> Qualcomm Nord platform.
>
> The platform includes two display clock controller instances,
> display0 and display1. Register support for both controllers.
>
> Signed-off-by: Taniya Das <taniya.das@oss.qualcomm.com>
> ---
> drivers/clk/qcom/Kconfig | 11 +
> drivers/clk/qcom/Makefile | 1 +
> drivers/clk/qcom/dispcc0-nord.c | 2006 +++++++++++++++++++++++++++++++++++++++
> drivers/clk/qcom/dispcc1-nord.c | 2006 +++++++++++++++++++++++++++++++++++++++
> 4 files changed, 4024 insertions(+)
>
> diff --git a/drivers/clk/qcom/Kconfig b/drivers/clk/qcom/Kconfig
> index 7d84c2f1d911a35430bba7670409f59972dcca0f..874136a2ad9aaa117df2c7ad5c8abc5280b76339 100644
> --- a/drivers/clk/qcom/Kconfig
> +++ b/drivers/clk/qcom/Kconfig
> @@ -145,6 +145,17 @@ config CLK_KAANAPALI_VIDEOCC
> Say Y if you want to support video devices and functionality such as
> video encode/decode.
>
> +config CLK_NORD_DISPCC
> + tristate "Nord Display Clock Controller"
> + depends on ARM64 || COMPILE_TEST
> + select CLK_NORD_GCC
> + default m if ARCH_QCOM
> + help
> + Support for the display clock controllers on Qualcomm Technologies, Inc
> + Nord devices. There are two display clock controllers on Nord SoC.
> + Say Y if you want to support display devices and functionality such as
> + splash screen.
> +
> config CLK_NORD_GCC
> tristate "Nord Global Clock Controller"
> depends on ARM64 || COMPILE_TEST
> diff --git a/drivers/clk/qcom/Makefile b/drivers/clk/qcom/Makefile
> index 58f9a5eb6fd7fc457607a179d8bab5623fedf706..4282f43e7078f1fe0dde6f942040eb6bd122d7ce 100644
> --- a/drivers/clk/qcom/Makefile
> +++ b/drivers/clk/qcom/Makefile
> @@ -37,6 +37,7 @@ obj-$(CONFIG_CLK_KAANAPALI_GCC) += gcc-kaanapali.o
> obj-$(CONFIG_CLK_KAANAPALI_GPUCC) += gpucc-kaanapali.o gxclkctl-kaanapali.o
> obj-$(CONFIG_CLK_KAANAPALI_TCSRCC) += tcsrcc-kaanapali.o
> obj-$(CONFIG_CLK_KAANAPALI_VIDEOCC) += videocc-kaanapali.o
> +obj-$(CONFIG_CLK_NORD_DISPCC) += dispcc0-nord.o dispcc1-nord.o
> obj-$(CONFIG_CLK_NORD_GCC) += gcc-nord.o negcc-nord.o nwgcc-nord.o segcc-nord.o
> obj-$(CONFIG_CLK_NORD_TCSRCC) += tcsrcc-nord.o
> obj-$(CONFIG_CLK_X1E80100_CAMCC) += camcc-x1e80100.o
> diff --git a/drivers/clk/qcom/dispcc0-nord.c b/drivers/clk/qcom/dispcc0-nord.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..c0097482a1a94a99a05767726b5a7405cf014fea
> --- /dev/null
> +++ b/drivers/clk/qcom/dispcc0-nord.c
> @@ -0,0 +1,2006 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
> + */
> +
> +#include <linux/clk-provider.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
This include shouldn't be needed.
Shawn
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
^ permalink raw reply
* Re: [PATCH 5/6] dt-bindings: clock: qcom: Document Nord GPU clock controllers
From: Shawn Guo @ 2026-06-24 2:52 UTC (permalink / raw)
To: Taniya Das
Cc: Bjorn Andersson, Michael Turquette, Stephen Boyd, Brian Masney,
Bartosz Golaszewski, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Neil Armstrong, Konrad Dybcio, Ajit Pandey,
Imran Shaik, Jagadeesh Kona, linux-arm-msm, linux-clk,
linux-kernel, devicetree
In-Reply-To: <20260623-nords_mm_v1-v1-5-860c84539804@oss.qualcomm.com>
On Tue, Jun 23, 2026 at 04:24:08PM +0530, Taniya Das wrote:
> Add Device Tree binding documentation for the GPU clock controllers
> on the Qualcomm Nord platform.
>
> The platform includes two GPU clock controller instances, GPUCC and
> GPUCC2. Document the compatible strings for both controllers.
>
> Signed-off-by: Taniya Das <taniya.das@oss.qualcomm.com>
> ---
> .../bindings/clock/qcom,sm8450-gpucc.yaml | 3 ++
> include/dt-bindings/clock/qcom,nord-gpucc.h | 51 ++++++++++++++++++++++
> 2 files changed, 54 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/clock/qcom,sm8450-gpucc.yaml b/Documentation/devicetree/bindings/clock/qcom,sm8450-gpucc.yaml
> index fdbdf605ee695637512ce4f98c9b6fcfacb9154f..ba85692240e07a4ed8e69c6b61847c5601b32fa0 100644
> --- a/Documentation/devicetree/bindings/clock/qcom,sm8450-gpucc.yaml
> +++ b/Documentation/devicetree/bindings/clock/qcom,sm8450-gpucc.yaml
> @@ -18,6 +18,7 @@ description: |
> include/dt-bindings/clock/qcom,glymur-gpucc.h
> include/dt-bindings/clock/qcom,kaanapali-gpucc.h
> include/dt-bindings/clock/qcom,milos-gpucc.h
> + include/dt-bindings/clock/qcom,nord-gpucc.h
> include/dt-bindings/clock/qcom,sar2130p-gpucc.h
> include/dt-bindings/clock/qcom,sm4450-gpucc.h
> include/dt-bindings/clock/qcom,sm8450-gpucc.h
> @@ -33,6 +34,8 @@ properties:
> - qcom,glymur-gpucc
> - qcom,kaanapali-gpucc
> - qcom,milos-gpucc
> + - qcom,nord-gpu2cc
> + - qcom,nord-gpucc
I see inconsistency in naming between dispcc0/dispcc1 and gpucc/gpu2cc.
Shawn
^ permalink raw reply
* Re: [PATCH v7 0/3] Add driver support for ESWIN EIC7700 HSP clock and reset generator
From: Xuyang Dong @ 2026-06-24 3:01 UTC (permalink / raw)
To: mturquette, sboyd, bmasney, benoit.monin
Cc: robh, krzk+dt, conor+dt, linux-clk, devicetree, linux-kernel,
p.zabel, huangyifeng, ningyu, linmin, pinkesh.vaghela
In-Reply-To: <20260605060730.1605-1-dongxuyang@eswincomputing.com>
>
> From: Xuyang Dong <dongxuyang@eswincomputing.com>
>
> Add support for the ESWIN EIC7700 HSP (high-speed peripherals). The drivers
> provide basic functionality to manage and control the clock and reset
> signals for EIC7700 HSP, including mmc, USB, ethernet, SATA and DMAC.
>
> The clock and reset registers are mapped to overlapping I/O address ranges.
> This causes a resource conflict when two drivers attempt to request the
> same region. Use the auxiliary device framework: the main driver
> allocates the shared register region and passes it to auxiliary
> devices, avoiding resource contention and duplicate remapping.
>
> Features:
> Implements support for the ESWIN EIC7700 HSP clock and reset controller.
> Provide API to manage clock and reset signals for the EIC7700 HSP.
>
> Supported chips:
> ESWIN EIC7700 series SoC.
>
> Test:
> Test this patch on the Sifive HiFive Premier P550 (which used the EIC7700
> SoC), include USB and other peripherals. All the drivers of these modules
> use the clock module and reset module.
>
Hi Stephen and Michael,
Just a gentle ping on this thread.
v7 now has Reviewed-by tags from Brian for the clock part, and from
Philipp for the reset part. Do you have any further comments on v7?
Best regards,
Xuyang Dong
> Updates:
> Changes in v7:
> - Clock driver:
> - Add the 'CLK_IGNORE_UNUSED' flag to all gate clocks; otherwise,
> board bring-up fails unless 'clk_ignore_unused' is passed on the
> kernel command line.
> - Sashiko's comments: not addressed in this version
> (see email for explanation)
>
> - Link to v6: https://lore.kernel.org/all/20260519081431.1424-1-dongxuyang@eswincomputing.com/
>
> Changes in v6:
> - Clock driver:
> - Add 'select REGMAP_MMIO' for Kconfig entry (Sashiko review of v5).
> - Reset driver:
> - Add <linux/module.h> (Sashiko review of v5).
>
> - Link to v5: https://lore.kernel.org/all/20260514114212.903-1-dongxuyang@eswincomputing.com/
>
> Changes in v5:
> - Clock driver:
> - Add check for regmap_read().
> - Add "Reviewed-by: Brian Masney <bmasney@redhat.com>".
>
> - Link to v4: https://lore.kernel.org/all/20260512020432.671-1-dongxuyang@eswincomputing.com/
>
> Changes in v4:
> - Clock driver:
> - Remove "Reviewed-by: Benoît Monin <benoit.monin@bootlin.com>" and
> "Reviewed-by: Brian Masney <bmasney@redhat.com>", because the clock
> driver has been updated.
> - Remove inclusion of io.h.
> - Add struct regmap to eic7700_hsp_clk_gate.
> Replace 'void __iomem *reg' with 'unsigned int reg'.
> Replace 'void __iomem *ref_reg' with 'unsigned int ref_reg'.
> Replace long with int for 'offset' and 'ref_offset'.
> Remove 'spinlock_t *lock'.
> Apply the same changes to hsp_clk_register_gate().
> - Remove the structure eic7700_hsp_regmap_lock, and the functions
> eic7700_hsp_regmap_lock() and eic7700_hsp_regmap_unlock().
> - Remove the 'guard(spinlock_irqsave)(gate->lock)' in
> hsp_clk_gate_endisable().
> - Replace readl() and writel() with regmap_assign_bits() in
> hsp_clk_gate_endisable().
> - Change the parameter enable from int to bool.
> - Replace readl() with regmap_read() in hsp_clk_gate_is_enabled().
> - Remove the lock_ctx variable.
> - Move eic7700_hsp_regmap_config from inside the probe function to global
> scope.
> Remove '.lock', '.unlock' and 'lock_arg'.
> Add '.fast_io = true' and '.use_raw_spinlock = true'.
> - Reset driver:
> - Add "Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>" for reset.
> - Drop the temporary variable, just return regmap_assign_bits(...) directly.
> - Replace -EINVAL with -ENODEV for dev_get_regmap() check.
> - Drop the '_dt'.
>
> - Link to v3: https://lore.kernel.org/all/20260423090904.2108-1-dongxuyang@eswincomputing.com/
>
> Changes in v3:
> - Bindings:
> - Added "Acked-by: Conor Dooley <conor.dooley@microchip.com>" for bindings.
> - Clock driver:
> - Remove 'gate_flags'.
> - Add __acquires for eic7700_hsp_regmap_lock() and add __releases for
> eic7700_hsp_regmap_unlock().
> - Move writel(USB_REF_XTAL24M, gate->ref_reg) into enable. Because this
> is only used for USB gate clock on the enable path. And modify the
> comments.
> - Simplify to: 'return !!(readl(gate->reg) & BIT(gate->bit_idx));'.
> - Drop const from eic7700_hsp_regmap_config.
> - Declare eic7700_hsp_regmap_config as a regular variable at the top.
>
> - Link to v2: https://lore.kernel.org/all/20260420093929.1895-1-dongxuyang@eswincomputing.com/
>
> Changes in v2:
> - Bindings:
> - Remove "hsp_" from clock-names.
> - Replace "eswin,eic7700-clock.yaml" and "eswin,eic7700-hspcrg.yaml" with
> "eswin,eic7700*".
> - Replace "eswin,eic7700-clock.h" and "eswin,eic7700-hspcrg.h" with
> "eswin,eic7700*".
> - Clock driver:
> - Use guard(spinlock_irqsave)(gate->lock) instead of spin_lock_irqsave()
> and remove spin_unlock_irqrestore().
> - Remove the newline in function hsp_clk_gate_is_enabled().
> - Use struct clk_init_data init = {}.
> - Replace 'static struct clk_parent_data' with
> 'static const struct clk_parent_data'.
> - Change '.fw_name' to '.index', because the function
> eswin_clk_register_fixed_factor() uses .index.
> - The structures of clocks should use static struct. When registering a clock,
> the 'hw' field in the structure will be assigned.
> - Remove __force.
> - Create the regmap in the clock driver and remove (__force void*)data->base.
> The reset driver uses dev_get_regmap() to get the regmap from the clock.
> - Move 'const struct regmap_config eic7700_hsp_regmap_config' from reset
> driver to clock driver.
> - The USB clock gate (hsp_clk_gate_endisable) and the reset driver both
> perform read-modify-write cycles on registers 0x800 and 0x900. Use
> custom regmap lock callbacks so that regmap operations hold data->lock
> with IRQs disabled, the same lock the clock gate path uses, preventing
> concurrent RMW races on those shared registers.
> - Change to 'ret = eswin_clk_register_fixed_factor(dev, eic7700_hsp_factor_clks,'.
> The next line will be over 80 characters and under 100 characters.
> - Reset driver:
> - Remove 'depends on COMMON_CLK_EIC7700_HSP' and 'default COMMON_CLK_EIC7700_HSP'.
> - Use regmap_assign_bits() in assert and deassert functions.
> - Remove eic7700_hsp_reset_reset().
> - The clock driver creates the regmap, and the reset driver uses dev_get_regmap().
> - Remove of_reset_n_cells.
>
> - Link to v1: https://lore.kernel.org/all/20260403093459.612-1-dongxuyang@eswincomputing.com/
>
> Xuyang Dong (3):
> dt-bindings: clock: Add ESWIN eic7700 HSP clock and reset generator
> clk: eswin: Add eic7700 HSP clock driver
> reset: eswin: Add eic7700 HSP reset driver
>
> .../bindings/clock/eswin,eic7700-hspcrg.yaml | 63 ++++
> MAINTAINERS | 5 +-
> drivers/clk/eswin/Kconfig | 13 +
> drivers/clk/eswin/Makefile | 1 +
> drivers/clk/eswin/clk-eic7700-hsp.c | 345 ++++++++++++++++++
> drivers/reset/Kconfig | 11 +
> drivers/reset/Makefile | 1 +
> drivers/reset/reset-eic7700-hsp.c | 113 ++++++
> .../dt-bindings/clock/eswin,eic7700-hspcrg.h | 33 ++
> .../dt-bindings/reset/eswin,eic7700-hspcrg.h | 21 ++
> 10 files changed, 604 insertions(+), 2 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/clock/eswin,eic7700-hspcrg.yaml
> create mode 100644 drivers/clk/eswin/clk-eic7700-hsp.c
> create mode 100644 drivers/reset/reset-eic7700-hsp.c
> create mode 100644 include/dt-bindings/clock/eswin,eic7700-hspcrg.h
> create mode 100644 include/dt-bindings/reset/eswin,eic7700-hspcrg.h
>
> --
> 2.34.1
^ permalink raw reply
* RE: [PATCH 0/7] soc: aspeed: Add AST2600 eSPI controller support
From: YH Chung @ 2026-06-24 3:59 UTC (permalink / raw)
To: Shulzhenko, Oleksandr, Arnd Bergmann, Andrew Jeffery,
Conor Dooley
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Joel Stanley,
Ryan Chen, Philipp Zabel, devicetree@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
linux-aspeed@lists.ozlabs.org, linux-kernel@vger.kernel.org,
openbmc@lists.ozlabs.org, maciej.lawniczak@intel.com, Mark Brown
In-Reply-To: <KL1PR0601MB4276452D09689C90B04209C190EF2@KL1PR0601MB4276.apcprd06.prod.outlook.com>
Hi Mark,
> (1) Reuse the existing SPI subsystem and treat eSPI packets as pure signals.
> (2) Maintain the driver under the SoC subsystem, since there is currently no
> eSPI subsystem.
> (3) Create a new eSPI subsystem and rewrite the eSPI driver accordingly.
>
> For option 1, we do not think this would be a good fit, because eSPI has clearly
> defined semantics for each channel, and our hardware exposes different sets of
> registers for each of them.
Regarding option 1, could you kindly share your feedback on whether this
patch set should be modified to fit under the SPI subsystem, or whether a new
eSPI subsystem would be more appropriate?
For option 2, Arnd has indicated that he does not want to take this through
the SoC subsystem. Therefore, if the SPI subsystem is also not a good fit, we
think a new eSPI subsystem may be needed to provide a common home for channel
handling, even though the individual channels are relatively independent.
For example, the VW and OOB channel handling could potentially be integrated
with existing subsystems such as GPIO or net/mctp. However, the Flash channel
would still need an eSPI-specific interface to configure the backing store.
Placing that code under MTD feels awkward, since the driver is not really an
MTD device or a flash controller.
If this direction sounds reasonable, we would like to explore introducing a
new drivers/espi subsystem and moving the eSPI driver there. Please let us
know if you think we should take a different approach.
Thanks,
Yun Hsuan.
^ permalink raw reply
* Re: [PATCH v2 3/3] iio: magnetometer: st_magn: honour st,fullscale-milligauss DT property
From: me @ 2026-06-24 4:18 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Jonathan Cameron, Herman van Hazendonk, David Lechner,
Nuno Sá, Andy Shevchenko, Nathan Chancellor,
Nick Desaulniers, Bill Wendling, Justin Stitt, Denis Ciocca,
Lars-Peter Clausen, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Denis Ciocca, Linus Walleij, linux-iio,
linux-kernel, llvm, devicetree
In-Reply-To: <ajrjRdEkZAho8h1E@ashevche-desk.local>
On 2026-06-23 21:49, Andy Shevchenko wrote:
> On Tue, Jun 23, 2026 at 08:29:16PM +0100, Jonathan Cameron wrote:
>> On Tue, 16 Jun 2026 15:02:06 +0200
>> Herman van Hazendonk <github.com@herrie.org> wrote:
>>
>> > The ST magnetometer core's common probe hardcodes fs_avl[0] -- the
>> > highest-sensitivity full-scale supported by the chip -- as the
>> > starting range. For the LSM303DLH that is +/-1.3 G; for the
>> > LSM303DLHC and LSM303DLM it is +/-2 G; for the LIS3MDL it is +/-4 G.
>> >
>> > That is the right default for "minimal noise floor at a desk", but
>> > it leaves no margin for boards that pick up appreciable DC bias from
>> > nearby PCB structures. On the HP TouchPad (apq8060 / tenderloin) the
>> > LSM303DLH magnetometer is mounted close enough to the surrounding
>> > power planes that X reads back as the chip's 0xF000 overflow
>> > sentinel (== -4096 raw, the value the chip publishes when the ADC
>> > saturates) on every sample at the chip-default range, while Y and Z
>> > fall well within the +/-1.3 G window.
>> >
>> > Parse the st,fullscale-milligauss device-tree property (documented
>> > separately in dt-bindings/iio/st,st-sensors.yaml) in the
>> > magnetometer common probe to select the initial fs_avl entry by its
>> > mg value. The DT binding pins the accepted value set per compatible
>> > via allOf/if-then enum clauses, so a malformed mg value fails
>> > dt_binding_check rather than reaching the driver. Sensors with a
>> > fixed full-scale (fs.addr == 0: LSM303AGR, LIS2MDL, IIS2MDC) have no
>> > register to switch and the property is rejected outright for them
>> > in the binding; the parse block is additionally gated on fs.addr as
>> > defence in depth against stale DTBs.
>> >
>> > Per-sensor mg ranges are listed in st_magn_sensors_settings[]. For
>> > LSM303DLH and LSM303DLHC/DLM the valid values are 1300, 1900, 2500,
>> > 4000, 4700, 5600 and 8100; for LIS3MDL, LSM9DS1-magn and LSM303C-magn
>> > they are 4000, 8000, 12000, 16000.
>> >
>> > Empirical scale sweep on the HP TouchPad confirmed that on this
>> > board any fs_avl >= 1 produces non-saturated X readings:
>> >
>> > scale (0.001 G/LSB) | X raw Y raw Z raw
>> > --------------------+-------------------------------
>> > 1.100 | -4096 44 46 (X saturated)
>> > 0.855 | -547 37 37 (clean)
>> > 0.670 | -433 94 103 (clean)
>> > 0.450 | -266 44 71 (clean)
>> > 0.400 | -235 34 65 (clean)
>> > 0.330 | -196 27 56 (clean)
>> > 0.230 | -145 15 40 (clean)
>> >
>> > 2500 mg is the natural choice for tenderloin: comfortably outside
>> > the saturation regime while keeping useful precision for compass
>> > applications.
>> >
>> > Assisted-by: Claude:claude-opus-4-7 sparse smatch clang-analyzer coccinelle checkpatch
>> > Assisted-by: Sashiko:claude-opus-4-7
>> Hmm. First time I remember seeing Sashiko credited like this. Seems
>> like pretty much
>> every patch series of any complexity would end up crediting sashiko.
>> Out of curiosity were you just looking at reports, or were you running
>> it locally to
>> help with development?
>
> I believe it's the second one, because LKML version uses Gemini (as far
> as I
> understand the case). At least that's why I haven't commented on this
> tag.
I have the whole toolchain running locally to avoid too many submissions
and
feedback from Sashiko with Gemini after submitting. For small drivers I
can run
Gemini locally as well, usually stick with Claude Opus 4.7 since that's
what I
have a subscription for. For very complicated and large drivers Claude
Opus
tends to time out even with 1 or 2 hour, so I fall back to Claude Haiku
4.5
(which catches quite a few things, but is not as thorough as Opus or
Gemini).
Seeing Sashiko tends to provide different feedback on different rounds,
I try
to only submit when Sashiko and all others are clean.
Hope this clarifies!
Herman
^ permalink raw reply
* Re: [PATCH v2 2/2] arm64: dts: qcom: sdm845-oneplus: Update compatible to include model
From: Dmitry Torokhov @ 2026-06-24 4:28 UTC (permalink / raw)
To: David Heidelberg
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Jason A. Donenfeld, Matthias Schiffer, Vincent Huang,
Bjorn Andersson, Konrad Dybcio, linux-input, devicetree,
linux-kernel, linux-arm-msm, phone-devel, Krzysztof Kozlowski,
Konrad Dybcio
In-Reply-To: <742c7a13-9465-40e8-8990-e679712e9784@ixit.cz>
Hi David,
On Sun, Jun 21, 2026 at 07:11:45PM +0200, David Heidelberg wrote:
> On 28/05/2026 00:13, David Heidelberg wrote:
> > On 27/05/2026 23:56, Dmitry Torokhov wrote:
> > > Hi David,
> > >
> > > On Sat, May 23, 2026 at 11:45:35AM +0200, David Heidelberg via B4 Relay wrote:
> > > > From: David Heidelberg <david@ixit.cz>
> > > >
> > > > We know the driver is reporting s3706b, introduce the compatible so we
> > > > can more easily introduce quirks for weird touchscreen replacements in
> > > > followup series.
> > > >
> > > > Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> > > > Signed-off-by: David Heidelberg <david@ixit.cz>
> > > > ---
> > > > arch/arm64/boot/dts/qcom/sdm845-oneplus-common.dtsi | 2 +-
> > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/arch/arm64/boot/dts/qcom/sdm845-oneplus-common.dtsi
> > > > b/arch/ arm64/boot/dts/qcom/sdm845-oneplus-common.dtsi
> > > > index 6b7378cf4d493..148164d456a5a 100644
> > > > --- a/arch/arm64/boot/dts/qcom/sdm845-oneplus-common.dtsi
> > > > +++ b/arch/arm64/boot/dts/qcom/sdm845-oneplus-common.dtsi
> > > > @@ -475,17 +475,17 @@ bq27441_fg: bq27441-battery@55 {
> > > > };
> > > > };
> > > > &i2c12 {
> > > > status = "okay";
> > > > clock-frequency = <400000>;
> > > > synaptics-rmi4-i2c@20 {
> > > > - compatible = "syna,rmi4-i2c";
> > > > + compatible = "syna,rmi4-s3706b", "syna,rmi4-i2c";
> > >
> > > So I believe we established that this device (s3706b) does not in fact
> > > implement rmi4 protocol properly. Why do we have "syna,rmi4-i2c" as a
> > > fallback? Shouldn't it be just "syna,rmi4-s3706b"?
> >
> > The vendor supplies s3706b which does implement the RMI4 properly.
> >
> > The 3rd party replacement impersonating original parts may not implement
> > it properly, but I don't address this issue in this initial submission.
> >
> > With this compatible we know which original part is used by the vendor
> > and installed in the phones, so later we can deduct specific sequences
> > for the replacement aftermarket parts to keep phone touchscreen working
> > same as they do on Android without affecting other devices.
>
> Hello Dmitry.
>
> May I ask what is currently preventing this series from moving forward?
>
> The first version was posted in 2023 [1]. I picked it up again in 2025 [2]
> and am now on the 9th iteration (this patchset). At this point, the series
> has been under discussion for well over a year, with relatively little
> feedback and increasingly long gaps between review rounds.
>
> The current approach is based on the guidance I have received so far,
> including suggestions from the device-tree maintainers. When concerns were
> raised, I tried to address them and rework the series accordingly.
>
> What I am struggling with is understanding what specific issue still needs
> to be resolved before these patches can be accepted. If there are remaining
> requirements, objections to the approach, or technical concerns that I have
> not addressed, I would appreciate having them stated explicitly so I can
> work on them.
>
> I also split out the straightforward, self-contained changes in the hope
> that at least those could progress independently while I continued working
> on any follow-up requirements. However, even those patches do not appear to
> be moving forward.
>
> Could you please clarify what outcome you would like to see from this
> series, and what concrete changes would be required to get it accepted?
I am still confused about how you want to differentiate between the full
RMI4 support vs the OnePlus flavor. The "syna,rmi4-s3706b", as you
mentioned, implements RMI4 protocol properly, so we do not need to
actually have it documented neither in binding nor in DTS.
The issue you have with after-market parts that are not compliant and we
need to figure out how to deal with them. Inside the driver I
essentially need a"incomplete protocol" flag that we can use to
implement additional checks or skip known to be not implemented
functions/queries. In DT we could introduce something like
"oneplus,rmi4-i2c" that is decidedly not compatible with "syna,rmi4-i2c"
and neither one should be a fallback for the other.
This of course needs buy-in from DT maintainers.
Does this make sense?
Thanks.
--
Dmitry
^ permalink raw reply
* [PATCH 0/2] Add AST2700 PECI clock support
From: Ryan Chen @ 2026-06-24 5:44 UTC (permalink / raw)
To: Michael Turquette, Stephen Boyd, Brian Masney, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Joel Stanley, Andrew Jeffery
Cc: linux-clk, devicetree, linux-arm-kernel, linux-aspeed,
linux-kernel, Ryan Chen
Add the PECI clock for the AST2700 SoC1. The PECI clock is a mux
controlled by SCU1_CLK_SEL2 bit 16, selecting between the 25MHz CLKIN
and HPLL/4.
Signed-off-by: Ryan Chen <ryan_chen@aspeedtech.com>
---
Ryan Chen (2):
dt-bindings: clock: ast2700: add PECI clock
clk: aspeed: add AST2700 PECI clock
drivers/clk/aspeed/clk-ast2700.c | 10 ++++++++++
include/dt-bindings/clock/aspeed,ast2700-scu.h | 2 ++
2 files changed, 12 insertions(+)
---
base-commit: 948efecf22e49aa4bf55bb73ec79a0ddcfd38571
change-id: 20260623-peci_clk-ba87e043ee5f
Best regards,
--
Ryan Chen <ryan_chen@aspeedtech.com>
^ permalink raw reply
* [PATCH 1/2] dt-bindings: clock: ast2700: add PECI clock
From: Ryan Chen @ 2026-06-24 5:44 UTC (permalink / raw)
To: Michael Turquette, Stephen Boyd, Brian Masney, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Joel Stanley, Andrew Jeffery
Cc: linux-clk, devicetree, linux-arm-kernel, linux-aspeed,
linux-kernel, Ryan Chen
In-Reply-To: <20260624-peci_clk-v1-0-ee28b92e22e9@aspeedtech.com>
Add SCU1_CLK_PECI for the SoC1 PECI controller clock source, and
SCU1_CLK_HPLL_DIV4 which serves as one of the PECI clock mux parents.
Signed-off-by: Ryan Chen <ryan_chen@aspeedtech.com>
---
include/dt-bindings/clock/aspeed,ast2700-scu.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/include/dt-bindings/clock/aspeed,ast2700-scu.h b/include/dt-bindings/clock/aspeed,ast2700-scu.h
index bacf712e8e04..138f78ce5f07 100644
--- a/include/dt-bindings/clock/aspeed,ast2700-scu.h
+++ b/include/dt-bindings/clock/aspeed,ast2700-scu.h
@@ -163,5 +163,7 @@
#define SCU1_CLK_GATE_PORTDUSB2CLK 85
#define SCU1_CLK_GATE_LTPI1TXCLK 86
#define SCU1_CLK_I3C 87
+#define SCU1_CLK_HPLL_DIV4 88
+#define SCU1_CLK_PECI 89
#endif
--
2.34.1
^ permalink raw reply related
* [PATCH 2/2] clk: aspeed: add AST2700 PECI clock
From: Ryan Chen @ 2026-06-24 5:45 UTC (permalink / raw)
To: Michael Turquette, Stephen Boyd, Brian Masney, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Joel Stanley, Andrew Jeffery
Cc: linux-clk, devicetree, linux-arm-kernel, linux-aspeed,
linux-kernel, Ryan Chen
In-Reply-To: <20260624-peci_clk-v1-0-ee28b92e22e9@aspeedtech.com>
Register the SoC1 PECI clock as a mux selected by SCU1_CLK_SEL2 bit 16,
choosing between the 25MHz CLKIN and HPLL/4. Add the HPLL/4 fixed factor
clock to serve as the second mux parent.
Signed-off-by: Ryan Chen <ryan_chen@aspeedtech.com>
---
drivers/clk/aspeed/clk-ast2700.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/drivers/clk/aspeed/clk-ast2700.c b/drivers/clk/aspeed/clk-ast2700.c
index 8b7b382f6f3e..5f499ec0e1d1 100644
--- a/drivers/clk/aspeed/clk-ast2700.c
+++ b/drivers/clk/aspeed/clk-ast2700.c
@@ -273,6 +273,13 @@ static const unsigned int sdclk_parent_ids[] = {
static const struct clk_hw *sdclk_parent_hws[ARRAY_SIZE(sdclk_parent_ids)];
+static const unsigned int peciclk_parent_ids[] = {
+ SCU1_CLKIN,
+ SCU1_CLK_HPLL_DIV4
+};
+
+static const struct clk_hw *peciclk_parent_hws[ARRAY_SIZE(peciclk_parent_ids)];
+
#define FIXED_CLK(_id, _name, _rate) \
{ \
.id = _id, \
@@ -458,6 +465,7 @@ static const struct ast2700_clk_info ast2700_scu1_clk_info[] __initconst = {
PLL_CLK(SCU1_CLK_HPLL, CLK_PLL, "soc1-hpll", SCU1_CLKIN, SCU1_HPLL_PARAM),
PLL_CLK(SCU1_CLK_APLL, CLK_PLL, "soc1-apll", SCU1_CLKIN, SCU1_APLL_PARAM),
PLL_CLK(SCU1_CLK_DPLL, CLK_PLL, "soc1-dpll", SCU1_CLKIN, SCU1_DPLL_PARAM),
+ FIXED_FACTOR_CLK(SCU1_CLK_HPLL_DIV4, "soc1-hpll_div4", SCU1_CLK_HPLL, 1, 4),
FIXED_FACTOR_CLK(SCU1_CLK_APLL_DIV2, "soc1-apll_div2", SCU1_CLK_APLL, 1, 2),
FIXED_FACTOR_CLK(SCU1_CLK_APLL_DIV4, "soc1-apll_div4", SCU1_CLK_APLL, 1, 4),
FIXED_FACTOR_CLK(SCU1_CLK_CAN, "canclk", SCU1_CLK_APLL, 1, 10),
@@ -481,6 +489,8 @@ static const struct ast2700_clk_info ast2700_scu1_clk_info[] __initconst = {
uxclk_parent_hws, SCU1_CLK_SEL2, 0, 2),
MUX_CLK(SCU1_CLK_HUXCLK, "huxclk", uxclk_parent_ids, ARRAY_SIZE(uxclk_parent_ids),
uxclk_parent_hws, SCU1_CLK_SEL2, 3, 2),
+ MUX_CLK(SCU1_CLK_PECI, "peciclk", peciclk_parent_ids, ARRAY_SIZE(peciclk_parent_ids),
+ peciclk_parent_hws, SCU1_CLK_SEL2, 16, 1),
DIVIDER_CLK(SCU1_CLK_SDCLK, "sdclk", SCU1_CLK_SDMUX,
SCU1_CLK_SEL1, 14, 3, ast2700_clk_div_table),
PLL_CLK(SCU1_CLK_UARTX, CLK_UART_PLL, "uartxclk", SCU1_CLK_UXCLK, SCU1_UXCLK_CTRL),
--
2.34.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox