Devicetree
 help / color / mirror / Atom feed
* Re: [PATCH v2] dt-bindings: pwm: st,pwm: convert to DT schema
From: sashiko-bot @ 2026-06-18 12:55 UTC (permalink / raw)
  To: Charan Pedumuru; +Cc: devicetree, robh, conor+dt
In-Reply-To: <20260618-st-pwm-v2-1-c792d5795ce2@gmail.com>

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

New issues:
- [High] The YAML binding strictly validates against `st,pwm`, but the hardware driver and actual device trees use `st,sti-pwm`.
- [Medium] The schema drops `pinctrl-0` from the required list but keeps `pinctrl-names` as required, creating an inconsistent configuration requirement.

Pre-existing issues:
- [High] The driver requests the IRQ before fully initializing the regmap fields used by the interrupt handler, creating a race condition that can lead to a NULL pointer dereference.
--

commit e38e89bd06d5e023d198bb6e9fb8add957cdf8e9
Author: Charan Pedumuru <charan.pedumuru@gmail.com>

dt-bindings: pwm: st,pwm: convert to DT schema

This commit converts the STMicroelectronics STiH41x PWM/Capture controller
text binding to DT schema. It updates the required list by adding
'interrupts' and dropping 'pinctrl-0'.

> diff --git a/Documentation/devicetree/bindings/pwm/st,pwm.yaml b/Documentation/devicetree/bindings/pwm/st,pwm.yaml
> new file mode 100644
> index 0000000000000..a045502e0e704
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pwm/st,pwm.yaml
> @@ -0,0 +1,93 @@
[ ... ]
> +properties:
> +  compatible:
> +    const: st,pwm

[Severity: High]
Does this compatible string match the hardware driver and actual device trees?

If existing in-kernel device trees and the matching driver
(drivers/pwm/pwm-sti.c) use "st,sti-pwm" instead of "st,pwm", will
make dtbs_check silently skip validation for them?

[ ... ]
> +  st,capture-num-chan:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: Number of available input capture channels.
> +    minimum: 0
> +    maximum: 4
> +    default: 0
> +
> +required:
> +  - reg
> +  - compatible
> +  - "#pwm-cells"
> +  - pinctrl-names

[Severity: Medium]
Since 'pinctrl-0' was dropped from the required list, should 'pinctrl-names'
also be removed?

Retaining 'pinctrl-names' in the required list while making 'pinctrl-0'
optional forces device trees that do not define any pinctrl configuration
to include a meaningless pinctrl-names = "default"; just to pass schema
validation.

> +  - clock-names
> +  - clocks
> +  - interrupts
[ ... ]
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    pwm@fe510000 {
> +        compatible = "st,pwm";

[Severity: High]
Does this example propagate an incorrect compatible string?

If the hardware actually uses "st,sti-pwm", using "st,pwm" here might mislead
future device tree authors.

[ ... ]

[Severity: High]
This is a pre-existing issue, but can the interrupt fire before the regmap
fields are fully initialized?

Looking at drivers/pwm/pwm-sti.c:sti_pwm_probe(), the IRQ is requested early:

	ret = devm_request_irq(&pdev->dev, irq, sti_pwm_interrupt, 0, ...);

Later, the regmap fields crucial for the IRQ handler are initialized:

	ret = sti_pwm_probe_regmap(pc);

If a pending interrupt from the bootloader fires immediately after
devm_request_irq(), sti_pwm_interrupt() will execute and call
regmap_field_read(pc->pwm_cpt_int_stat, ...). Since pc->pwm_cpt_int_stat
is NULL before sti_pwm_probe_regmap() initializes it, would this cause a
kernel panic due to a NULL pointer dereference?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260618-st-pwm-v2-1-c792d5795ce2@gmail.com?part=1

^ permalink raw reply

* Re: [PATCH v8 2/7] mfd: Add driver for ASUS Transformer embedded controller
From: Svyatoslav Ryhel @ 2026-06-18 12:54 UTC (permalink / raw)
  To: Lee Jones
  Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Dmitry Torokhov,
	Pavel Machek, Sebastian Reichel, Ion Agorria,
	Michał Mirosław, devicetree, linux-kernel, linux-input,
	linux-leds, linux-pm
In-Reply-To: <20260618122605.GH1672911@google.com>

чт, 18 черв. 2026 р. о 15:26 Lee Jones <lee@kernel.org> пише:
>
> On Thu, 11 Jun 2026, Svyatoslav Ryhel wrote:
>
> > чт, 11 черв. 2026 р. о 14:17 Lee Jones <lee@kernel.org> пише:
> > >
> > > On Thu, 28 May 2026, Svyatoslav Ryhel wrote:
> > > > From: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> > > >
> > > > Support Nuvoton NPCE795-based ECs as used in Asus Transformer TF201,
> > > > TF300T, TF300TG, TF300TL and TF700T pad and dock, as well as TF101 dock
> > > > and TF600T, P1801-T and TF701T pad. This is a glue driver handling
> > > > detection and common operations for EC's functions.
> > > >
> > > > Co-developed-by: Svyatoslav Ryhel <clamor95@gmail.com>
> > > > Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> > > > Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> > > > ---
> > > >  drivers/mfd/Kconfig                     |  16 +
> > > >  drivers/mfd/Makefile                    |   1 +
> > > >  drivers/mfd/asus-transformer-ec.c       | 542 ++++++++++++++++++++++++
> > > >  include/linux/mfd/asus-transformer-ec.h |  92 ++++
> > > >  4 files changed, 651 insertions(+)
> > > >  create mode 100644 drivers/mfd/asus-transformer-ec.c
> > > >  create mode 100644 include/linux/mfd/asus-transformer-ec.h
>
> [...]
>
> > > > +static void asus_ec_clear_buffer(struct asus_ec_data *ddata)
> > > > +{
> > > > +     int ret, retry = ASUSEC_RSP_BUFFER_SIZE;
> > > > +
> > > > +     /*
> > > > +      * Read the buffer till we get valid data by checking ASUSEC_OBF_MASK
> > > > +      * of the status byte or till we reach end of the 256 byte buffer.
> > > > +      */
> > > > +     while (retry--) {
> > > > +             ret = i2c_smbus_read_i2c_block_data(ddata->client, ASUSEC_READ_BUF,
> > > > +                                                 ASUSEC_ENTRY_SIZE,
> > > > +                                                 ddata->ec_buf);
> > > > +             if (ret < ASUSEC_ENTRY_SIZE)
> > > > +                     continue;
> > > > +
> > > > +             if (ddata->ec_buf[ASUSEC_IRQ_STATUS] & ASUSEC_OBF_MASK)
> > > > +                     continue;
> > > > +
> > > > +             break;
> > > > +     }
> > > > +}
> > > > +
> > > > +static int asus_ec_log_info(struct asus_ec_data *ddata, unsigned int reg,
> > > > +                         const char *name, const char **out)
>
> If we can avoid points to pointers, then please do.
>
> We already have ddata, so we can just set the name?
>
> It will remove a lot of the following complexity / ugliness.
>
> > > > +{
> > > > +     struct device *dev = &ddata->client->dev;
> > > > +     u8 buf[ASUSEC_ENTRY_BUFSIZE];
> > > > +     int ret;
> > > > +
> > > > +     memset(buf, 0, ASUSEC_ENTRY_BUFSIZE);
> > > > +     ret = i2c_smbus_read_i2c_block_data(ddata->ec.dockram, reg,
> > > > +                                         ASUSEC_ENTRY_SIZE, buf);
> > > > +     if (ret < ASUSEC_ENTRY_SIZE)
> > > > +             return ret;
> > >
> > > Same here.  These should be negative.
> > >
> >
> > return ret < 0 ? ret : -EIO same as above
> >
> > > > +
> > > > +     if (buf[0] > ASUSEC_ENTRY_SIZE) {
> > > > +             dev_err(dev, "bad data len; buffer: %*ph; ret: %d\n",
> > > > +                     ASUSEC_ENTRY_BUFSIZE, buf, ret);
> > > > +             return -EPROTO;
> > > > +     }
> > > > +
> > > > +     if (!ddata->logging_disabled) {
> > > > +             dev_info(dev, "%-14s: %.*s\n", name, buf[0], buf + 1);
> > > > +
> > > > +             if (out) {
> > > > +                     *out = devm_kasprintf(dev, GFP_KERNEL, "%.*s",
> > > > +                                           buf[0], buf + 1);
> > > > +                     if (!*out)
> > > > +                             return -ENOMEM;
> > > > +             }
> > > > +     }
> > >
> > > FWIW, I hate this!  What does it give you now that development is done?
> > >
> >
> > We have already discussed this, and you agreed that EC and firmware
> > prints may stay! This prints EC model and firmware info as well as EC
> > firmware behavior. It allows identify possible new revisions of EC -
> > Firmware combo and address possible regressions (check if it is chip
> > malfunction or firmware needs a new programming model) without
> > rebuilding kernel and digging downstream kernel for needed bits of
> > code.
>
> Right, so just print it out and remove all of the 'logging_disabled' and
> 'out' nonsense.
>
> > > > +     return 0;
> > > > +}
> > > > +
> > > > +static int asus_ec_reset(struct asus_ec_data *ddata)
> > > > +{
> > > > +     int retry, ret;
> > > > +
> > > > +     guard(mutex)(&ddata->ecreq_lock);
> > > > +
> > > > +     for (retry = 0; retry < ASUSEC_RETRY_MAX; retry++) {
> > >
> > > for (int retry = ... is generally preferred for throwaway variables.
> > >
> >
> > Not that I care too much, but I am defining ret anyway, why not add
> > retry too there?
>
> This is the new and preferred way to use throw-away variables.
>
> ret is not a throw-away variable.
>
> [...]
>
> > > > +static int asus_ec_set_factory_mode(struct asus_ec_data *ddata,
> > > > +                                 enum asusec_mode fmode)
> > > > +{
> > > > +     dev_info(&ddata->client->dev, "Entering %s mode.\n",
> > > > +              fmode == ASUSEC_MODE_FACTORY ? "factory" : "normal");
> > > > +
> > > > +     return asus_dockram_access_ctl(ddata->ec.dockram, NULL,
> > > > +                                    ASUSEC_CTL_FACTORY_MODE,
> > > > +                                    fmode == ASUSEC_MODE_FACTORY ?
> > > > +                                    ASUSEC_CTL_FACTORY_MODE : 0);
> > >
> > > Why not create make:
> > >
> > > ASUSEC_MODE_FACTORY == ASUSEC_CTL_FACTORY_MODE
> > >
> > > What happens to NORMAL?
> > >
> >
> > ASUSEC_CTL_FACTORY_MODE is a bit in the ctl register. For NORMAL mode
>
> I get that, but if the values can be shared, it make the code simpler.
>
> > bit is cleared,
> > for FACTORY bit it set, for NONE bit is ignored.
> >
> > > > +}
> > > > +
> > > > +static int asus_ec_detect(struct asus_ec_data *ddata)
> > > > +{
> > > > +     int ret;
> > > > +
> > > > +     ret = asus_ec_reset(ddata);
> > > > +     if (ret)
> > > > +             goto err_exit;
> > > > +
> > > > +     asus_ec_clear_buffer(ddata);
> > > > +
> > > > +     ret = asus_ec_log_info(ddata, ASUSEC_DOCKRAM_INFO_MODEL, "Model",
> > > > +                            &ddata->ec.model);
>
> Where is this model used?
>

Model is passed to cells to form names, particularly input device names.

> > > You can use 100-chars and make the code look beautiful! :)
> > >
> >
> > Not every subsystem permits 100 chars, some stick to 80 as a strict
> > rule, so it is better be safe.
>
> Right, but we are forward thinking here.
>
> You can and should use 100-chars in this subsystem.
>
> > > > +     if (ret)
> > > > +             goto err_exit;
> > > > +
> > > > +     ret = asus_ec_log_info(ddata, ASUSEC_DOCKRAM_INFO_FW, "FW version",
> > > > +                            NULL);
> > > > +     if (ret)
> > > > +             goto err_exit;
> > > > +
> > > > +     ret = asus_ec_log_info(ddata, ASUSEC_DOCKRAM_INFO_CFGFMT, "Config format",
> > > > +                            NULL);
> > > > +     if (ret)
> > > > +             goto err_exit;
> > > > +
> > > > +     ret = asus_ec_log_info(ddata, ASUSEC_DOCKRAM_INFO_HW, "HW version",
> > > > +                            NULL);
> > > > +     if (ret)
> > > > +             goto err_exit;
> > > > +
> > > > +     /* Disable logging on next EC request */
> > >
> > > Why, but why?
> > >
> >
> > Cause EC requests are frequent (handshake/reset) and constant logging
> > same data is not acceptable.
>
> Then rid the prints entirely or do them at a more appropriate point like
> during probe?
>
> Or maybe consider dev_info_once() and friends.
>

I totally forgot about dev_info_once(), thank you.

> > > > +     ddata->logging_disabled = true;
> > > > +
> > > > +     /* Check and inform about EC firmware behavior */
> > > > +     ret = asus_ec_susb_on_status(ddata);
> > > > +     if (ret)
> > > > +             goto err_exit;
> > > > +
> > > > +     ddata->ec.name = ddata->info->name;
> > > > +
> > > > +     /* Some EC require factory mode to be set normal on each request */
> > > > +     if (ddata->info->fmode)
> > > > +             ret = asus_ec_set_factory_mode(ddata, ddata->info->fmode);
> > > > +
> > > > +err_exit:
> > > > +     if (ret)
> > > > +             dev_err(&ddata->client->dev, "failed to access EC: %d\n", ret);
> > > > +
> > > > +     return ret;
> > > > +}
> > > > +
> > > > +static void asus_ec_handle_smi(struct asus_ec_data *ddata, unsigned int code)
> > > > +{
> > > > +     switch (code) {
> > > > +     case ASUSEC_SMI_HANDSHAKE:
> > > > +     case ASUSEC_SMI_RESET:
> > > > +             asus_ec_detect(ddata);
> > > > +             break;
> > > > +     }
> > > > +}
> > > > +
> > > > +static irqreturn_t asus_ec_interrupt(int irq, void *dev_id)
> > > > +{
> > > > +     struct asus_ec_data *ddata = dev_id;
> > > > +     unsigned long notify_action;
> > > > +     int ret;
> > > > +
> > > > +     ret = i2c_smbus_read_i2c_block_data(ddata->client, ASUSEC_READ_BUF,
> > > > +                                         ASUSEC_ENTRY_SIZE, ddata->ec_buf);
> > > > +     if (ret < ASUSEC_ENTRY_SIZE ||
> > > > +         !(ddata->ec_buf[ASUSEC_IRQ_STATUS] & ASUSEC_OBF_MASK))
> > >
> > > Unwrap for readability.
> > >
> > > Also, I think a comment would be helpful.
> > >
> >
> > if (ret < ASUSEC_ENTRY_SIZE)
> >     return IRQ_NONE;
> >
> > ret = ddata->ec_buf[ASUSEC_IRQ_STATUS] & ASUSEC_OBF_MASK;
> > if (!ret)
> >     return IRQ_NONE;
> >
> > This would be acceptable? (I will add comments later on)
>
> Yes, better.
>
> If you're not using ret again, you could just put 'ddata.." inside the if().
>

I thought about this, but that would require combining it with (!)
which will not help with readabilty.

> > > > +             return IRQ_NONE;
> > > > +
> > > > +     notify_action = ddata->ec_buf[ASUSEC_IRQ_STATUS];
> > > > +     if (notify_action & ASUSEC_SMI_MASK) {
> > > > +             unsigned int code = ddata->ec_buf[ASUSEC_SMI_CODE];
> > > > +
> > > > +             asus_ec_handle_smi(ddata, code);
> > > > +
> > > > +             notify_action |= code << 8;
> > > > +     }
> > > > +
> > > > +     blocking_notifier_call_chain(&ddata->ec.notify_list,
> > > > +                                  notify_action, ddata->ec_buf);
> > > > +
> > > > +     return IRQ_HANDLED;
> > > > +}
> > > > +
> > > > +static void asus_ec_release_dockram_dev(void *client)
> > > > +{
> > > > +     i2c_unregister_device(client);
> > > > +}
> > > > +
> > > > +static struct i2c_client *devm_asus_dockram_get(struct device *dev)
> > > > +{
> > > > +     struct i2c_client *parent = to_i2c_client(dev);
> > > > +     struct i2c_client *dockram;
> > > > +     struct dockram_ec_data *ddata;
> > > > +     int ret;
> > > > +
> > > > +     dockram = i2c_new_ancillary_device(parent, "dockram",
> > > > +                                        parent->addr + 2);
> > >
> > > Could we define a macro for the address offset '2' here to avoid using a magic
> > > number?
> > >
> >
> > It seems that you are excessively concerned with "magic numbers".
>
> Bingo!  I HATE magic numbers.
>
> https://lore.kernel.org/all/?q=%22Lee+Jones%22+magic
>
> ~900 messages!  =:-D
>

AHAHAH, ok, this makes things clearer. Lemmy have a note about this quirk.

> [...]
>
> > > > +static const struct asus_ec_chip_info asus_ec_tf600t_pad_data = {
> > > > +     .name = "pad",
> > > > +     .variant = ASUSEC_TF600T_PAD,
> > > > +     .fmode = ASUSEC_MODE_NORMAL,
> > > > +};
> > >
> > > Any reason not to just pass the identifier (variant) and add the name
> > > and fmode attribues to the switch() above?
> >
> > Why not set it here, I am not passing any mfd or any other API via of data.
>
> I get that, and you're not breaking any of my golden rules.
>
> However, I just think doing everything in one place, usually a which
> based off of the 'variant' which you pass as a single value, is a nicer,
> more consolidated way of doing things.
>

Well, I would need to pass OF data regardless, so why not bundle it
with all required info and leave probes switch for strictly mfd cell
assignment. I suppose this is more a personal preference issue. I hope
you will not mind if I leave this as it is?

All other comments you have left and I did not answer directly were
read and acknowledged. Thank you!

> --
> Lee Jones

^ permalink raw reply

* Re: [PATCH v11 5/9] mfd: p3h2x4x: Add driver for NXP P3H2x4x i3c hub and on-die regulator
From: Lee Jones @ 2026-06-18 12:48 UTC (permalink / raw)
  To: Lakshay Piplani
  Cc: linux-kernel, linux-i3c, alexandre.belloni, krzk+dt, robh,
	conor+dt, devicetree, broonie, Frank.Li, lgirdwood, vikash.bansal,
	priyanka.jain, aman.kumarpandey
In-Reply-To: <20260612111816.3688240-6-lakshay.piplani@nxp.com>

/* Sashiko Automation: Issues Found (3 Findings) */

Please investigate.

> From: Aman Kumar Pandey <aman.kumarpandey@nxp.com>
> 
> Add core MFD support for the NXP P3H2x4x (P3H2440/P3H2441/P3H2840/P3H2841)
> family of multiport I3C hub devices. These devices connect to a host via
> I3C/I2C/SMBus and expose multiple downstream target ports.
> 
> Signed-off-by: Aman Kumar Pandey <aman.kumarpandey@nxp.com>
> Signed-off-by: Vikash Bansal <vikash.bansal@nxp.com>
> Signed-off-by: Lakshay Piplani <lakshay.piplani@nxp.com>
> 
> ---
> Changes in v11:
>  - Use MFD_CELL_NAME() for child device registration
>  - Rename local variables for consistency
>  - Rename driver names to follow subsystem conventions:
>    - Use '-' instead of '_' in driver names
>    - Drop the "_drv" suffix from driver names
> 
> Changes in v10:
>  - Drop redundant is_p3h2x4x_in_i3c flag
> 
> Changes in v9:
>  - Renamed macros to follow consistent uppercase naming conventions
>  - Made REGMAP selects in the P3H2X4X MFD Kconfig conditional,
>    to avoid I3C/I2C dependency issues
> 
> Changes in v8:
>  - No change
> 
> Changes in v7:
>  - Use new config I3C_OR_I2C
> 
> Changes in v6:
>  - No change
> 
> Changes in v5:
>  - Corrected the ordering in the Makefile and Kconfig for MFD_P3H2X4X
>  - Updated dev_err_probe() for regmap_init failure.
>  - Updated module description
> 
> Changes in v4:
>  - Split the driver into three separate patches(mfd, regulator and I3C hub)
>  - Added support for NXP P3H2x4x MFD functionality
> ---
> ---
>  MAINTAINERS                 |   2 +
>  drivers/mfd/Kconfig         |  13 ++++
>  drivers/mfd/Makefile        |   1 +
>  drivers/mfd/p3h2840.c       | 119 ++++++++++++++++++++++++++++++++++++
>  include/linux/i3c/device.h  |   1 +
>  include/linux/mfd/p3h2840.h |  26 ++++++++
>  6 files changed, 162 insertions(+)
>  create mode 100644 drivers/mfd/p3h2840.c
>  create mode 100644 include/linux/mfd/p3h2840.h
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index ba65ae5a008b..3420701a75c5 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -19311,6 +19311,8 @@ L:	linux-kernel@vger.kernel.org
>  L:	linux-i3c@lists.infradead.org
>  S:	Maintained
>  F:	Documentation/devicetree/bindings/i3c/nxp,p3h2840.yaml
> +F:	drivers/mfd/p3h2840.c
> +F:	include/linux/mfd/p3h2840.h
>  
>  NXP PF5300/PF5301/PF5302 PMIC REGULATOR DEVICE DRIVER
>  M:	Woodrow Douglass <wdouglass@carnegierobotics.com>
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index 7192c9d1d268..405b50c3c77b 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -617,6 +617,19 @@ config MFD_MX25_TSADC
>  	  i.MX25 processors. They consist of a conversion queue for general
>  	  purpose ADC and a queue for Touchscreens.
>  
> +config MFD_P3H2X4X
> +	tristate "NXP P3H2X4X I3C Hub Device"
> +	depends on I3C_OR_I2C
> +	select MFD_CORE
> +	select REGMAP_I3C if I3C
> +	select REGMAP_I2C if I2C
> +	help
> +	  Enable Support for NXP P3H244x/P3H284x I3C HUB device using I3C/I2C
> +	  communication interface.
> +
> +	  This driver provides support for I3C hub and regulator, each subdriver
> +	  can be enabled independently depending on the required functionality.
> +
>  config MFD_PF1550
>  	tristate "NXP PF1550 PMIC Support"
>  	depends on I2C=y && OF
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index e75e8045c28a..a284b22c7b13 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -122,6 +122,7 @@ obj-$(CONFIG_MFD_MC13XXX)	+= mc13xxx-core.o
>  obj-$(CONFIG_MFD_MC13XXX_SPI)	+= mc13xxx-spi.o
>  obj-$(CONFIG_MFD_MC13XXX_I2C)	+= mc13xxx-i2c.o
>  
> +obj-$(CONFIG_MFD_P3H2X4X)	+= p3h2840.o
>  obj-$(CONFIG_MFD_PF1550)	+= pf1550.o
>  
>  obj-$(CONFIG_MFD_NCT6694)	+= nct6694.o
> diff --git a/drivers/mfd/p3h2840.c b/drivers/mfd/p3h2840.c
> new file mode 100644
> index 000000000000..0a80992d6250
> --- /dev/null
> +++ b/drivers/mfd/p3h2840.c
> @@ -0,0 +1,119 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright 2025-2026 NXP
> + * P3H2X4X i3c hub and regulator device.
> + */
> +
> +#include <linux/i2c.h>
> +#include <linux/i3c/master.h>

[Severity: Low]
Does this include bypass the I3C subsystem abstraction? By including the
master header, the driver has access to the internal definition of
struct i3c_device. Should this include <linux/i3c/device.h> instead?



> +#include <linux/mfd/core.h>
> +#include <linux/mfd/p3h2840.h>
> +#include <linux/regmap.h>
> +
> +static const struct mfd_cell p3h2x4x_devs[] = {
> +		MFD_CELL_NAME("p3h2x4x-regulator"),
> +		MFD_CELL_NAME("p3h2x4x-i3c-hub"),
> +};
> +
> +static const struct regmap_config p3h2x4x_regmap_config = {
> +	.reg_bits = P3H2X4X_REG_BITS,
> +	.val_bits = P3H2X4X_VAL_BITS,
> +	.max_register = 0xFF,
> +};
> +
> +static int p3h2x4x_device_probe_i3c(struct i3c_device *i3cdev)
> +{
> +	struct p3h2x4x_dev *ddata;
> +	int ret;
> +
> +	ddata = devm_kzalloc(&i3cdev->dev, sizeof(*ddata), GFP_KERNEL);

[Severity: Low]
This directly accesses the internal dev field of struct i3c_device. Should
this use i3cdev_to_dev(i3cdev) to obtain the underlying struct device pointer
to maintain API boundaries?



> +	if (!ddata)
> +		return -ENOMEM;
> +
> +	i3cdev_set_drvdata(i3cdev, ddata);
> +
> +	ddata->regmap = devm_regmap_init_i3c(i3cdev, &p3h2x4x_regmap_config);
> +	if (IS_ERR(ddata->regmap))
> +		return dev_err_probe(&i3cdev->dev, PTR_ERR(ddata->regmap),
> +				     "Failed to register HUB regmap\n");
> +
> +	// The hub child driver retrieves information from i3cdev
> +	ddata->i3cdev = i3cdev;
> +
> +	ret = devm_mfd_add_devices(&i3cdev->dev, PLATFORM_DEVID_AUTO,
> +				   p3h2x4x_devs, ARRAY_SIZE(p3h2x4x_devs),
> +				   NULL, 0, NULL);
> +	if (ret)
> +		return dev_err_probe(&i3cdev->dev, ret, "Failed to add sub devices\n");
> +
> +	return 0;
> +}
> +
> +static int p3h2x4x_device_probe_i2c(struct i2c_client *client)
> +{
> +	struct p3h2x4x_dev *ddata;
> +	int ret;
> +
> +	ddata = devm_kzalloc(&client->dev, sizeof(*ddata), GFP_KERNEL);
> +	if (!ddata)
> +		return -ENOMEM;
> +
> +	i2c_set_clientdata(client, ddata);
> +
> +	ddata->regmap = devm_regmap_init_i2c(client, &p3h2x4x_regmap_config);
> +	if (IS_ERR(ddata->regmap))
> +		return dev_err_probe(&client->dev, PTR_ERR(ddata->regmap),
> +				     "Failed to register HUB regmap\n");
> +
> +	ddata->i3cdev = NULL;
> +
> +	ret = devm_mfd_add_devices(&client->dev, PLATFORM_DEVID_AUTO,
> +				   p3h2x4x_devs, ARRAY_SIZE(p3h2x4x_devs),
> +				   NULL, 0, NULL);
> +	if (ret)
> +		return dev_err_probe(&client->dev, ret, "Failed to add sub devices\n");
> +
> +	return 0;
> +}
> +
> +static const struct i3c_device_id p3h2x4x_i3c_ids[] = {
> +	I3C_CLASS(I3C_DCR_HUB, NULL),
> +	{ /* sentinel */ },
> +};

[Severity: High]
Will this driver inappropriately bind to any generic I3C hub device on the
bus?

Using I3C_CLASS(I3C_DCR_HUB, NULL) instructs the subsystem to match any
device that advertises the generic Hub DCR (194), without filtering by
vendor or part ID.

Because the probe function doesn't verify the Manufacturer ID, it seems this
could eagerly bind to third-party hubs and attempt NXP-specific register
operations on them.

Should this use I3C_DEVICE(I3C_VENDOR_NXP, ...) to exclusively match the
supported NXP devices instead of the generic hub class?

> +MODULE_DEVICE_TABLE(i3c, p3h2x4x_i3c_ids);
> +
> +static const struct i2c_device_id p3h2x4x_i2c_id_table[] = {
> +	{ "nxp-i3c-hub" },
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(i2c, p3h2x4x_i2c_id_table);
> +
> +static const struct of_device_id p3h2x4x_i2c_of_match[] = {
> +	{ .compatible = "nxp,p3h2840", },
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, p3h2x4x_i2c_of_match);
> +
> +static struct i3c_driver p3h2x4x_i3c = {
> +	.driver = {
> +		.name = "p3h2x4x-i3c",
> +	},
> +	.probe = p3h2x4x_device_probe_i3c,
> +	.id_table = p3h2x4x_i3c_ids,
> +};
> +
> +static struct i2c_driver p3h2x4x_i2c = {
> +	.driver = {
> +		.name = "p3h2x4x-i2c",
> +		.of_match_table = p3h2x4x_i2c_of_match,
> +	},
> +	.probe =  p3h2x4x_device_probe_i2c,
> +	.id_table = p3h2x4x_i2c_id_table,
> +};
> +module_i3c_i2c_driver(p3h2x4x_i3c, &p3h2x4x_i2c);
> +
> +MODULE_AUTHOR("Aman Kumar Pandey <aman.kumarpandey@nxp.com>");
> +MODULE_AUTHOR("Vikash Bansal <vikash.bansal@nxp.com>");
> +MODULE_AUTHOR("Lakshay Piplani <lakshay.piplani@nxp.com>");
> +MODULE_DESCRIPTION("NXP P3H2X4X I3C HUB multi function driver");
> +MODULE_LICENSE("GPL");
> diff --git a/include/linux/i3c/device.h b/include/linux/i3c/device.h
> index 971d53349b6f..6188082599dd 100644
> --- a/include/linux/i3c/device.h
> +++ b/include/linux/i3c/device.h
> @@ -85,6 +85,7 @@ struct i3c_xfer {
>   */
>  enum i3c_dcr {
>  	I3C_DCR_GENERIC_DEVICE = 0,
> +	I3C_DCR_HUB = 194,
>  };
>  
>  #define I3C_PID_MANUF_ID(pid)		(((pid) & GENMASK_ULL(47, 33)) >> 33)
> diff --git a/include/linux/mfd/p3h2840.h b/include/linux/mfd/p3h2840.h
> new file mode 100644
> index 000000000000..2d4fae2176ae
> --- /dev/null
> +++ b/include/linux/mfd/p3h2840.h
> @@ -0,0 +1,26 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright 2025-2026 NXP
> + * This header file contains register definitions and bit masks for the P3H2X4X.
> + */
> +
> +#ifndef _LINUX_MFD_P3H2840_H
> +#define _LINUX_MFD_P3H2840_H
> +
> +#include <linux/types.h>
> +
> +/* Device Configuration Registers */
> +#define P3H2X4X_DEV_REG_PROTECTION_CODE				0x10
> +#define P3H2X4X_REGISTERS_LOCK_CODE				0x00
> +#define P3H2X4X_REGISTERS_UNLOCK_CODE				0x69
> +#define P3H2X4X_CP1_REGISTERS_UNLOCK_CODE			0x6a
> +
> +/* Reg config for Regmap */
> +#define P3H2X4X_REG_BITS					8
> +#define P3H2X4X_VAL_BITS					8
> +
> +struct p3h2x4x_dev {
> +	struct i3c_device *i3cdev;
> +	struct regmap *regmap;
> +};
> +#endif /* _LINUX_MFD_P3H2840_H */
> -- 
> 2.25.1
> 

-- 
Lee Jones

^ permalink raw reply

* [PATCH v2] arm64: dts: qcom: qcom6490-idp: Add IPA nodes
From: Sarat Addepalli @ 2026-06-18 12:47 UTC (permalink / raw)
  To: andersson, konradybcio, robh, krzk+dt, conor+dt, linux-arm-msm,
	devicetree, linux-kernel, pavan.m
  Cc: Sarat Addepalli

Add IPA nodes for Qualcomm qcm6490 board.

Signed-off-by: Sarat Addepalli <sarat.addepalli@oss.qualcomm.com>
---
 arch/arm64/boot/dts/qcom/qcm6490-idp.dts | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/qcm6490-idp.dts b/arch/arm64/boot/dts/qcom/qcm6490-idp.dts
index bdc02260f902..fd1aa26e20f1 100644
--- a/arch/arm64/boot/dts/qcom/qcm6490-idp.dts
+++ b/arch/arm64/boot/dts/qcom/qcm6490-idp.dts
@@ -619,6 +619,13 @@ &gpu_zap_shader {
 	firmware-name = "qcom/qcm6490/a660_zap.mbn";
 };
 
+&ipa {
+	qcom,gsi-loader = "self";
+	memory-region = <&ipa_fw_mem>;
+	firmware-name = "qcom/qcm6490/ipa_fws.mbn";
+	status = "okay";
+};
+
 &lpass_rx_macro {
 	status = "okay";
 };

^ permalink raw reply related

* Re: [PATCH v2 2/2] drm/panel: add Ilitek ILI7807S panel driver
From: Neil Armstrong @ 2026-06-18 12:43 UTC (permalink / raw)
  To: Arpit Saini, Jessica Zhang, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley
  Cc: linux-arm-msm, dri-devel, devicetree, linux-kernel,
	ayushi.makhija, rajeevny
In-Reply-To: <20260618-ili7807s-v2-2-b3f0c109b102@oss.qualcomm.com>

On 6/18/26 12:24, Arpit Saini wrote:
> Add a DRM panel driver for the DLC DLC0697 1080x1920@60Hz MIPI DSI
> panel based on the Ilitek ILI7807S display controller.
> 
> The panel operates in video burst mode with four data lanes using
> RGB888 pixel format.
> 
> Signed-off-by: Arpit Saini <arpit.saini@oss.qualcomm.com>
> ---
>   drivers/gpu/drm/panel/Kconfig                 |  12 ++
>   drivers/gpu/drm/panel/Makefile                |   1 +
>   drivers/gpu/drm/panel/panel-ilitek-ili7807s.c | 297 ++++++++++++++++++++++++++
>   3 files changed, 310 insertions(+)
> 
> diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
> index 7450b27622a2..1cbaac1bf545 100644
> --- a/drivers/gpu/drm/panel/Kconfig
> +++ b/drivers/gpu/drm/panel/Kconfig
> @@ -264,6 +264,18 @@ config DRM_PANEL_HYDIS_HV101HD1
>   
>   	  If M is selected the module will be called panel-hydis-hv101hd1
>   
> +config DRM_PANEL_ILITEK_ILI7807S
> +	tristate "Ilitek ILI7807S-based panels"
> +	depends on OF
> +	depends on DRM_MIPI_DSI
> +	depends on BACKLIGHT_CLASS_DEVICE
> +	help
> +	  Say Y if you want to enable support for panels based on the
> +	  Ilitek ILI7807S display controller, such as the DLC DLC0697
> +	  1080x1920 MIPI DSI panel.
> +
> +	  If M is selected the module will be called panel-ilitek-ili7807s.
> +
>   config DRM_PANEL_ILITEK_IL9322
>   	tristate "Ilitek ILI9322 320x240 QVGA panels"
>   	depends on OF && SPI
> diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
> index c2c5cf817116..c3002b351cb8 100644
> --- a/drivers/gpu/drm/panel/Makefile
> +++ b/drivers/gpu/drm/panel/Makefile
> @@ -26,6 +26,7 @@ obj-$(CONFIG_DRM_PANEL_HIMAX_HX83112B) += panel-himax-hx83112b.o
>   obj-$(CONFIG_DRM_PANEL_HIMAX_HX83121A) += panel-himax-hx83121a.o
>   obj-$(CONFIG_DRM_PANEL_HIMAX_HX8394) += panel-himax-hx8394.o
>   obj-$(CONFIG_DRM_PANEL_HYDIS_HV101HD1) += panel-hydis-hv101hd1.o
> +obj-$(CONFIG_DRM_PANEL_ILITEK_ILI7807S) += panel-ilitek-ili7807s.o
>   obj-$(CONFIG_DRM_PANEL_ILITEK_IL9322) += panel-ilitek-ili9322.o
>   obj-$(CONFIG_DRM_PANEL_ILITEK_ILI9341) += panel-ilitek-ili9341.o
>   obj-$(CONFIG_DRM_PANEL_ILITEK_ILI9805) += panel-ilitek-ili9805.o
> diff --git a/drivers/gpu/drm/panel/panel-ilitek-ili7807s.c b/drivers/gpu/drm/panel/panel-ilitek-ili7807s.c
> new file mode 100644
> index 000000000000..bfbb8e29a4aa
> --- /dev/null
> +++ b/drivers/gpu/drm/panel/panel-ilitek-ili7807s.c
> @@ -0,0 +1,297 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
> + */
> +
> +#include <linux/backlight.h>
> +#include <linux/delay.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/regulator/consumer.h>
> +
> +#include <drm/drm_mipi_dsi.h>
> +#include <drm/drm_modes.h>
> +#include <drm/drm_panel.h>
> +#include <drm/drm_probe_helper.h>
> +
> +struct panel_desc {
> +	const struct drm_display_mode *mode;
> +	unsigned int lanes;
> +	enum mipi_dsi_pixel_format format;
> +	unsigned long mode_flags;
> +	void (*init)(struct mipi_dsi_multi_context *dsi_ctx);
> +};
> +
> +struct ili7807s {
> +	struct drm_panel panel;
> +	struct mipi_dsi_device *dsi;
> +	const struct panel_desc *desc;
> +
> +	struct regulator_bulk_data *supplies;
> +	struct gpio_desc *reset_gpio;
> +};
> +
> +static const struct regulator_bulk_data ili7807s_supplies[] = {
> +	{ .supply = "vddi" },
> +	{ .supply = "avdd" },
> +	{ .supply = "avee" },
> +};
> +
> +static inline struct ili7807s *to_ili7807s(struct drm_panel *panel)
> +{
> +	return container_of(panel, struct ili7807s, panel);
> +}
> +
> +static void ili7807s_reset(struct ili7807s *ctx)
> +{
> +	gpiod_set_value_cansleep(ctx->reset_gpio, 0);
> +	usleep_range(10000, 11000);
> +	gpiod_set_value_cansleep(ctx->reset_gpio, 1);
> +	usleep_range(10000, 11000);
> +	gpiod_set_value_cansleep(ctx->reset_gpio, 0);
> +	usleep_range(10000, 11000);
> +}
> +
> +static void dlc0697_init_sequence(struct mipi_dsi_multi_context *dsi_ctx)
> +{
> +	mipi_dsi_dcs_soft_reset_multi(dsi_ctx);
> +	mipi_dsi_msleep(dsi_ctx, 120);
> +
> +	mipi_dsi_dcs_write_seq_multi(dsi_ctx, 0xff, 0x78, 0x07, 0x00);
> +	mipi_dsi_dcs_write_seq_multi(dsi_ctx, 0x35, 0x00);
> +	mipi_dsi_dcs_write_seq_multi(dsi_ctx, 0x5e, 0x09, 0x99);
> +	mipi_dsi_dcs_write_seq_multi(dsi_ctx, 0x53, 0x24);
> +	mipi_dsi_dcs_write_seq_multi(dsi_ctx, 0x55, 0x01);
> +	mipi_dsi_dcs_write_seq_multi(dsi_ctx, 0x51, 0x3f, 0xff);
> +
> +	mipi_dsi_dcs_exit_sleep_mode_multi(dsi_ctx);
> +	mipi_dsi_msleep(dsi_ctx, 120);
> +
> +	mipi_dsi_dcs_set_display_on_multi(dsi_ctx);
> +	mipi_dsi_msleep(dsi_ctx, 20);
> +}
> +
> +static int ili7807s_on(struct ili7807s *ctx)
> +{
> +	struct mipi_dsi_multi_context dsi_ctx = { .dsi = ctx->dsi };
> +
> +	ctx->dsi->mode_flags |= MIPI_DSI_MODE_LPM;
> +
> +	ctx->desc->init(&dsi_ctx);
> +
> +	ctx->dsi->mode_flags &= ~MIPI_DSI_MODE_LPM;
> +
> +	return dsi_ctx.accum_err;
> +}
> +
> +static int ili7807s_off(struct ili7807s *ctx)
> +{
> +	struct mipi_dsi_multi_context dsi_ctx = { .dsi = ctx->dsi };
> +
> +	ctx->dsi->mode_flags |= MIPI_DSI_MODE_LPM;
> +
> +	mipi_dsi_dcs_set_display_off_multi(&dsi_ctx);
> +	mipi_dsi_msleep(&dsi_ctx, 20);
> +
> +	mipi_dsi_dcs_enter_sleep_mode_multi(&dsi_ctx);
> +	mipi_dsi_msleep(&dsi_ctx, 120);
> +
> +	ctx->dsi->mode_flags &= ~MIPI_DSI_MODE_LPM;
> +
> +	return dsi_ctx.accum_err;
> +}
> +
> +static int ili7807s_prepare(struct drm_panel *panel)
> +{
> +	struct ili7807s *ctx = to_ili7807s(panel);
> +	int ret;
> +
> +	ret = regulator_bulk_enable(ARRAY_SIZE(ili7807s_supplies), ctx->supplies);
> +	if (ret < 0) {
> +		dev_err(ctx->panel.dev, "failed to enable regulators: %d\n", ret);
> +		return ret;
> +	}
> +
> +	msleep(20);
> +
> +	ili7807s_reset(ctx);
> +
> +	ret = ili7807s_on(ctx);
> +	if (ret < 0) {
> +		dev_err(ctx->panel.dev, "failed to initialise panel: %d\n", ret);
> +		goto err;
> +	}
> +
> +	return 0;
> +
> +err:
> +	gpiod_set_value_cansleep(ctx->reset_gpio, 1);
> +
> +	regulator_bulk_disable(ARRAY_SIZE(ili7807s_supplies), ctx->supplies);
> +	return ret;
> +}
> +
> +static int ili7807s_unprepare(struct drm_panel *panel)
> +{
> +	struct ili7807s *ctx = to_ili7807s(panel);
> +	int ret;
> +
> +	ret = ili7807s_off(ctx);
> +	if (ret < 0)
> +		dev_err(ctx->panel.dev, "failed to disable panel: %d\n", ret);
> +
> +	gpiod_set_value_cansleep(ctx->reset_gpio, 1);
> +
> +	regulator_bulk_disable(ARRAY_SIZE(ili7807s_supplies), ctx->supplies);
> +
> +	return 0;
> +}
> +
> +static int ili7807s_get_modes(struct drm_panel *panel,
> +			      struct drm_connector *connector)
> +{
> +	struct ili7807s *ctx = to_ili7807s(panel);
> +
> +	return drm_connector_helper_get_modes_fixed(connector, ctx->desc->mode);
> +}
> +
> +static const struct drm_panel_funcs ili7807s_panel_funcs = {
> +	.prepare = ili7807s_prepare,
> +	.unprepare = ili7807s_unprepare,
> +	.get_modes = ili7807s_get_modes,
> +};
> +
> +static int ili7807s_bl_update_status(struct backlight_device *bl)
> +{
> +	struct mipi_dsi_device *dsi = bl_get_data(bl);
> +	u16 brightness = backlight_get_brightness(bl);
> +	int ret;
> +
> +	dsi->mode_flags &= ~MIPI_DSI_MODE_LPM;
> +
> +	ret = mipi_dsi_dcs_set_display_brightness_large(dsi, brightness);
> +
> +	dsi->mode_flags |= MIPI_DSI_MODE_LPM;
> +
> +	return ret;
> +}
> +
> +static const struct backlight_ops ili7807s_bl_ops = {
> +	.update_status = ili7807s_bl_update_status,
> +};
> +
> +static struct backlight_device *ili7807s_create_backlight(struct mipi_dsi_device *dsi)
> +{
> +	struct device *dev = &dsi->dev;
> +	const struct backlight_properties props = {
> +		.type           = BACKLIGHT_RAW,
> +		.brightness     = 0x3fff,
> +		.max_brightness = 0x3fff,
> +	};
> +
> +	return devm_backlight_device_register(dev, dev_name(dev), dev, dsi,
> +					      &ili7807s_bl_ops, &props);
> +}
> +
> +static const struct drm_display_mode dlc0697_mode = {
> +	.clock = 131911,

	.clock = (1080 + 18 + 2 + 16) * (1920 + 26 + 4 + 20) * 60 / 1000,
> +
> +	.hdisplay    = 1080,
> +	.hsync_start = 1080 + 18,
> +	.hsync_end   = 1080 + 18 + 2,
> +	.htotal      = 1080 + 18 + 2 + 16,
> +
> +	.vdisplay    = 1920,
> +	.vsync_start = 1920 + 26,
> +	.vsync_end   = 1920 + 26 + 4,
> +	.vtotal      = 1920 + 26 + 4 + 20,
> +
> +	.width_mm  = 0,
> +	.height_mm = 0,
> +	.type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED,
> +};
> +
> +static const struct panel_desc dlc0697_desc = {
> +	.mode       = &dlc0697_mode,
> +	.lanes      = 4,
> +	.format     = MIPI_DSI_FMT_RGB888,
> +	.mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_BURST,
> +	.init       = dlc0697_init_sequence,
> +};
> +
> +static int ili7807s_probe(struct mipi_dsi_device *dsi)
> +{
> +	struct device *dev = &dsi->dev;
> +	const struct panel_desc *desc;
> +	struct ili7807s *ctx;
> +	int ret;
> +
> +	ctx = devm_drm_panel_alloc(dev, struct ili7807s, panel,
> +				   &ili7807s_panel_funcs,
> +				   DRM_MODE_CONNECTOR_DSI);
> +	if (IS_ERR(ctx))
> +		return PTR_ERR(ctx);
> +
> +	desc = of_device_get_match_data(dev);
> +	ctx->desc = desc;
> +
> +	ret = devm_regulator_bulk_get_const(dev, ARRAY_SIZE(ili7807s_supplies),
> +					    ili7807s_supplies, &ctx->supplies);
> +	if (ret < 0)
> +		return dev_err_probe(dev, ret, "failed to get regulators\n");
> +
> +	ctx->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
> +	if (IS_ERR(ctx->reset_gpio))
> +		return dev_err_probe(dev, PTR_ERR(ctx->reset_gpio),
> +				     "failed to get reset gpio\n");
> +
> +	ctx->dsi = dsi;
> +	mipi_dsi_set_drvdata(dsi, ctx);
> +
> +	dsi->lanes      = desc->lanes;
> +	dsi->format     = desc->format;
> +	dsi->mode_flags = desc->mode_flags;
> +
> +	ctx->panel.prepare_prev_first = true;
> +
> +	ret = drm_panel_of_backlight(&ctx->panel);
> +	if (ret)
> +		return ret;
> +
> +	if (!ctx->panel.backlight) {
> +		ctx->panel.backlight = ili7807s_create_backlight(dsi);
> +		if (IS_ERR(ctx->panel.backlight))
> +			return dev_err_probe(dev, PTR_ERR(ctx->panel.backlight),
> +					     "failed to create backlight\n");
> +	}
> +
> +	ret = devm_drm_panel_add(dev, &ctx->panel);
> +	if (ret < 0)
> +		return dev_err_probe(dev, ret, "failed to add panel\n");
> +
> +	ret = devm_mipi_dsi_attach(dev, dsi);
> +	if (ret < 0)
> +		return dev_err_probe(dev, ret, "failed to attach dsi\n");
> +
> +	return 0;

Return devm_mipi_dsi_attach() directly

> +}
> +
> +static const struct of_device_id ili7807s_of_match[] = {
> +	{ .compatible = "dlc,dlc0697", .data = &dlc0697_desc },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, ili7807s_of_match);
> +
> +static struct mipi_dsi_driver ili7807s_dsi_driver = {
> +	.probe  = ili7807s_probe,
> +	.driver = {
> +		.name           = "panel-ilitek-ili7807s",
> +		.of_match_table = ili7807s_of_match,
> +	},
> +};
> +module_mipi_dsi_driver(ili7807s_dsi_driver);
> +
> +MODULE_AUTHOR("Arpit Saini <arpit.saini@oss.qualcomm.com>");
> +MODULE_DESCRIPTION("Panel driver for Ilitek ILI7807S LCD DSI panel");
> +MODULE_LICENSE("GPL");
> 

Thanks,
Neil

^ permalink raw reply

* [PATCH v2] dt-bindings: pwm: st,pwm: convert to DT schema
From: Charan Pedumuru @ 2026-06-18 12:42 UTC (permalink / raw)
  To: Uwe Kleine-König, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Pal Singh
  Cc: linux-pwm, devicetree, linux-kernel, Charan Pedumuru

Convert STMicroelectronics STiH41x PWM/Capture controller binding
to DT schema.
Changes during conversion:
- Add 'interrupts' to the required list. The text binding listed it
  as required and the driver unconditionally calls platform_get_irq().
- Drop pinctrl-0 from the required list as pinctrl properties are
  inherited and validated by the pinctrl schema.

Signed-off-by: Charan Pedumuru <charan.pedumuru@gmail.com>
---
Changes in v2:
- Add the missing interrupts to the required following the old binding.
- Modify commit message to explain the changes made.
- Link to v1: https://patch.msgid.link/20260613-st-pwm-v1-1-458c2c89709a@gmail.com
---
 Documentation/devicetree/bindings/pwm/pwm-st.txt  | 43 -----------
 Documentation/devicetree/bindings/pwm/st,pwm.yaml | 93 +++++++++++++++++++++++
 2 files changed, 93 insertions(+), 43 deletions(-)

diff --git a/Documentation/devicetree/bindings/pwm/pwm-st.txt b/Documentation/devicetree/bindings/pwm/pwm-st.txt
deleted file mode 100644
index 19fce774cafa..000000000000
--- a/Documentation/devicetree/bindings/pwm/pwm-st.txt
+++ /dev/null
@@ -1,43 +0,0 @@
-STMicroelectronics PWM driver bindings
---------------------------------------
-
-Required parameters:
-- compatible :		"st,pwm"
-- #pwm-cells : 		Number of cells used to specify a PWM. First cell
-			specifies the per-chip index of the PWM to use and the
-			second cell is the period in nanoseconds - fixed to 2
-			for STiH41x.
-- reg :			Physical base address and length of the controller's
-			registers.
-- pinctrl-names: 	Set to "default".
-- pinctrl-0: 		List of phandles pointing to pin configuration nodes
-			for PWM module.
-			For Pinctrl properties, please refer to [1].
-- clock-names: 		Valid entries are "pwm" and/or "capture".
-- clocks: 		phandle of the clock used by the PWM module.
-			For Clk properties, please refer to [2].
-- interrupts:		IRQ for the Capture device
-
-Optional properties:
-- st,pwm-num-chan:	Number of available PWM channels.  Default is 0.
-- st,capture-num-chan:	Number of available Capture channels.  Default is 0.
-
-[1] Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
-[2] Documentation/devicetree/bindings/clock/clock-bindings.txt
-
-Example:
-
-pwm1: pwm@fe510000 {
-	compatible = "st,pwm";
-	reg = <0xfe510000 0x68>;
-	#pwm-cells = <2>;
-	pinctrl-names = "default";
-	pinctrl-0 = <&pinctrl_pwm1_chan0_default
-		     &pinctrl_pwm1_chan1_default
-		     &pinctrl_pwm1_chan2_default
-		     &pinctrl_pwm1_chan3_default>;
-	clocks = <&clk_sysin>;
-	clock-names = "pwm";
-	st,pwm-num-chan = <4>;
-	st,capture-num-chan = <2>;
-};
diff --git a/Documentation/devicetree/bindings/pwm/st,pwm.yaml b/Documentation/devicetree/bindings/pwm/st,pwm.yaml
new file mode 100644
index 000000000000..a045502e0e70
--- /dev/null
+++ b/Documentation/devicetree/bindings/pwm/st,pwm.yaml
@@ -0,0 +1,93 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pwm/st,pwm.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: STMicroelectronics STiH41x PWM/Capture controller
+
+maintainers:
+  - Ajit Pal Singh <ajitpal.singh@st.com>
+
+description:
+  The STiH41x PWM controller supports both PWM output and input capture
+  functionality. It provides multiple PWM output channels for generating
+  variable duty-cycle waveforms, and multiple input capture channels for
+  measuring external signal periods and pulse widths. PWM output channels
+  and input capture channels are configured independently via
+  st,pwm-num-chan and st,capture-num-chan respectively.
+
+allOf:
+  - $ref: pwm.yaml#
+
+properties:
+  compatible:
+    const: st,pwm
+
+  reg:
+    maxItems: 1
+
+  "#pwm-cells":
+    const: 2
+
+  pinctrl-names:
+    const: default
+
+  clock-names:
+    items:
+      enum: [pwm, capture]
+    minItems: 1
+    maxItems: 2
+
+  clocks:
+    minItems: 1
+    maxItems: 2
+
+  interrupts:
+    description: IRQ line for the capture device.
+    maxItems: 1
+
+  st,pwm-num-chan:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: Number of available PWM output channels.
+    minimum: 0
+    maximum: 4
+    default: 0
+
+  st,capture-num-chan:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: Number of available input capture channels.
+    minimum: 0
+    maximum: 4
+    default: 0
+
+required:
+  - reg
+  - compatible
+  - "#pwm-cells"
+  - pinctrl-names
+  - clock-names
+  - clocks
+  - interrupts
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    pwm@fe510000 {
+        compatible = "st,pwm";
+        reg = <0xfe510000 0x68>;
+        interrupts = <GIC_SPI 128 IRQ_TYPE_LEVEL_HIGH>;
+        #pwm-cells = <2>;
+        pinctrl-names = "default";
+        pinctrl-0 = <&pinctrl_pwm1_chan0_default
+                     &pinctrl_pwm1_chan1_default
+                     &pinctrl_pwm1_chan2_default
+                     &pinctrl_pwm1_chan3_default>;
+        clocks = <&clk_sysin>;
+        clock-names = "pwm";
+        st,pwm-num-chan = <4>;
+        st,capture-num-chan = <2>;
+    };
+...

---
base-commit: c425609d6ac4012c8bbf01ec2e10e801b1923a7b
change-id: 20260613-st-pwm-1feade6cfea3

Best regards,
--  
Charan Pedumuru <charan.pedumuru@gmail.com>


^ permalink raw reply related

* Re: [PATCH 3/4] mfd: Add support for UGREEN NASync DH2300 MCU
From: Lee Jones @ 2026-06-18 12:40 UTC (permalink / raw)
  To: Alexey Charkov
  Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Heiko Stuebner,
	Liam Girdwood, Mark Brown, devicetree, linux-kernel,
	linux-arm-kernel, linux-rockchip
In-Reply-To: <20260612-dh2300-mcu-v1-3-ab8db1617bc0@flipper.net>

On Fri, 12 Jun 2026, Alexey Charkov wrote:

> Add a driver for the HC32F005 MCU used as an embedded controller on the
> UGREEN NASync DH2300 NAS.
> 
> This part provides the shared I2C regmap to be used by function-specific
> sub-devices, and instantiates the SATA drive-bay power gate regulator.
> Implemented as an MFD to allow for other functions of the MCU to be added
> later: vendor binaries imply that it also provides a hardware watchdog
> and somehow serves as a wake source, but so far only the SATA power gating
> function has been confirmed in absence of documentation and sources for the
> vendor firmware.
> 
> Signed-off-by: Alexey Charkov <alchark@flipper.net>
> ---
>  MAINTAINERS                     |  1 +
>  drivers/mfd/Kconfig             | 16 +++++++++++
>  drivers/mfd/Makefile            |  1 +
>  drivers/mfd/ugreen-dh2300-mcu.c | 60 +++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 78 insertions(+)

Did you see: drivers/mfd/simple-mfd-i2c.c ?

> diff --git a/MAINTAINERS b/MAINTAINERS
> index ca27df7cd684..9578a06fe651 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -27637,6 +27637,7 @@ UGREEN DH2300 MCU MFD DRIVER
>  M:	Alexey Charkov <alchark@flipper.net>
>  S:	Maintained
>  F:	Documentation/devicetree/bindings/mfd/ugreen,dh2300-mcu.yaml
> +F:	drivers/mfd/ugreen-dh2300-mcu.c
>  
>  UHID USERSPACE HID IO DRIVER
>  M:	David Rheinsberg <david@readahead.eu>
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index 763ce6a34782..5a2ad75bd9c9 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -1947,6 +1947,22 @@ config MFD_TPS6594_SPI
>  	  This driver can also be built as a module.  If so, the module
>  	  will be called tps6594-spi.
>  
> +config MFD_UGREEN_DH2300_MCU
> +	tristate "UGREEN NASync DH2300 embedded controller"
> +	depends on I2C
> +	depends on OF
> +	select MFD_CORE
> +	select REGMAP_I2C
> +	help
> +	  Say yes here to enable support for the HC32F005 microcontroller found
> +	  on the UGREEN NASync DH2300 NAS, where it acts as a board embedded
> +	  controller. This core driver sets up the shared register map and
> +	  instantiates the function sub-devices (the SATA drive-bay power
> +	  regulator).
> +
> +	  This driver can also be built as a module. If so, the module will be
> +	  called ugreen-dh2300-mcu.
> +
>  config TWL4030_CORE
>  	bool "TI TWL4030/TWL5030/TWL6030/TPS659x0 Support"
>  	depends on I2C=y
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index dd4bb7e77c33..6247239bcfe1 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -109,6 +109,7 @@ obj-$(CONFIG_MFD_TPS65912_SPI)  += tps65912-spi.o
>  obj-$(CONFIG_MFD_TPS6594)	+= tps6594-core.o
>  obj-$(CONFIG_MFD_TPS6594_I2C)	+= tps6594-i2c.o
>  obj-$(CONFIG_MFD_TPS6594_SPI)	+= tps6594-spi.o
> +obj-$(CONFIG_MFD_UGREEN_DH2300_MCU)	+= ugreen-dh2300-mcu.o
>  obj-$(CONFIG_MENELAUS)		+= menelaus.o
>  
>  obj-$(CONFIG_TWL4030_CORE)	+= twl-core.o twl4030-irq.o twl6030-irq.o
> diff --git a/drivers/mfd/ugreen-dh2300-mcu.c b/drivers/mfd/ugreen-dh2300-mcu.c
> new file mode 100644
> index 000000000000..5184b0c98759
> --- /dev/null
> +++ b/drivers/mfd/ugreen-dh2300-mcu.c
> @@ -0,0 +1,60 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Core driver for the UGREEN NASync DH2300 embedded controller (HC32F005 MCU).
> + *
> + * The microcontroller sits on I2C and exposes an 8-bit register map. It is a
> + * multi-function device: SATA drive-bay power gate, hardware watchdog and
> + * possibly other functions
> + */
> +
> +#include <linux/i2c.h>
> +#include <linux/mfd/core.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/module.h>
> +#include <linux/regmap.h>
> +
> +#define UGREEN_DH2300_MCU_REG_MAX	0x94
> +
> +static const struct regmap_config ugreen_dh2300_mcu_regmap_config = {
> +	.reg_bits = 8,
> +	.val_bits = 8,
> +	.max_register = UGREEN_DH2300_MCU_REG_MAX,
> +};
> +
> +static const struct mfd_cell ugreen_dh2300_mcu_cells[] = {
> +	{ .name = "ugreen-dh2300-mcu-regulator" },
> +};
> +
> +static int ugreen_dh2300_mcu_probe(struct i2c_client *client)
> +{
> +	struct device *dev = &client->dev;
> +	struct regmap *regmap;
> +
> +	regmap = devm_regmap_init_i2c(client, &ugreen_dh2300_mcu_regmap_config);
> +	if (IS_ERR(regmap))
> +		return dev_err_probe(dev, PTR_ERR(regmap),
> +				     "failed to initialise regmap\n");
> +
> +	return devm_mfd_add_devices(dev, PLATFORM_DEVID_AUTO,
> +				    ugreen_dh2300_mcu_cells,
> +				    ARRAY_SIZE(ugreen_dh2300_mcu_cells),
> +				    NULL, 0, NULL);
> +}
> +
> +static const struct of_device_id ugreen_dh2300_mcu_of_match[] = {
> +	{ .compatible = "ugreen,dh2300-mcu" },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, ugreen_dh2300_mcu_of_match);
> +
> +static struct i2c_driver ugreen_dh2300_mcu_driver = {
> +	.driver = {
> +		.name = "ugreen-dh2300-mcu",
> +		.of_match_table = ugreen_dh2300_mcu_of_match,
> +	},
> +	.probe = ugreen_dh2300_mcu_probe,
> +};
> +module_i2c_driver(ugreen_dh2300_mcu_driver);
> +
> +MODULE_DESCRIPTION("UGREEN NASync DH2300 embedded controller core driver");
> +MODULE_LICENSE("GPL");
> 
> -- 
> 2.53.0
> 

-- 
Lee Jones

^ permalink raw reply

* Re: [PATCH 2/2] media: i2c: og0va1b: Add OmniVision OG0VA1B camera sensor
From: Wenmeng Liu @ 2026-06-18 12:36 UTC (permalink / raw)
  To: Vladimir Zapolskiy, Mauro Carvalho Chehab, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Sakari Ailus
  Cc: linux-media, devicetree, linux-kernel
In-Reply-To: <1a57863c-831a-411c-a0ae-da3d4f1fd6a0@linaro.org>



On 6/18/2026 6:50 PM, Vladimir Zapolskiy wrote:
> Hello Wenmeng.
> 
> On 6/18/26 13:37, Wenmeng Liu wrote:
>> Add V4L2 sub device driver for OmniVision OG0VA1B image sensor.
>> OmniVision OG0VA1B is an image sensor, which produces frames in 10-bit
>> raw output format (Y10) over a 1-lane MIPI CSI-2 interface and supports
>> the 640x480 (VGA) resolution.
>>
>> Signed-off-by: Wenmeng Liu <wenmeng.liu@oss.qualcomm.com>
>> ---
>>   MAINTAINERS                 |   1 +
>>   drivers/media/i2c/Kconfig   |  10 +
>>   drivers/media/i2c/Makefile  |   1 +
>>   drivers/media/i2c/og0va1b.c | 867 ++++++++++++++++++++++++++++++++++ 
>> ++++++++++
>>   4 files changed, 879 insertions(+)
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 
>> 5aa846c8479b20651291d5bd2e316308310f826c..85a06eb9eacc410a565b80d56979eaa565515d0e 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -19891,6 +19891,7 @@ M:    Wenmeng Liu <wenmeng.liu@oss.qualcomm.com>
>>   L:    linux-media@vger.kernel.org
>>   S:    Maintained
>>   F:    Documentation/devicetree/bindings/media/i2c/ovti,og0va1b.yaml
>> +F:    drivers/media/i2c/og0va1b.c
>>   OMNIVISION OG0VE1B SENSOR DRIVER
>>   M:    Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
>> diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
>> index 
>> 5d173e0ecf424f2f204f8d426be818e44357f8e4..56680772f5f47b4629c4e17f5a5feba08b1d94fc 100644
>> --- a/drivers/media/i2c/Kconfig
>> +++ b/drivers/media/i2c/Kconfig
>> @@ -363,6 +363,16 @@ config VIDEO_OG01A1B
>>         To compile this driver as a module, choose M here: the
>>         module will be called og01a1b.
>> +config VIDEO_OG0VA1B
>> +    tristate "OmniVision OG0VA1B sensor support"
>> +    select V4L2_CCI_I2C
>> +    help
>> +      This is a Video4Linux2 sensor driver for the OmniVision
>> +      OG0VA1B camera.
>> +
>> +      To compile this driver as a module, choose M here: the
>> +      module will be called og0va1b.
>> +
>>   config VIDEO_OG0VE1B
>>       tristate "OmniVision OG0VE1B sensor support"
>>       select V4L2_CCI_I2C
>> diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
>> index 
>> e45359efe0e41e13e3c0869e5ead7d6cf4aca3a7..c60851c7fe07e3bdc511c5f482525ba7a044f48e 100644
>> --- a/drivers/media/i2c/Makefile
>> +++ b/drivers/media/i2c/Makefile
>> @@ -83,6 +83,7 @@ obj-$(CONFIG_VIDEO_MT9V011) += mt9v011.o
>>   obj-$(CONFIG_VIDEO_MT9V032) += mt9v032.o
>>   obj-$(CONFIG_VIDEO_MT9V111) += mt9v111.o
>>   obj-$(CONFIG_VIDEO_OG01A1B) += og01a1b.o
>> +obj-$(CONFIG_VIDEO_OG0VA1B) += og0va1b.o
>>   obj-$(CONFIG_VIDEO_OG0VE1B) += og0ve1b.o
>>   obj-$(CONFIG_VIDEO_OS05B10) += os05b10.o
>>   obj-$(CONFIG_VIDEO_OV01A10) += ov01a10.o
>> diff --git a/drivers/media/i2c/og0va1b.c b/drivers/media/i2c/og0va1b.c
>> new file mode 100644
>> index 
>> 0000000000000000000000000000000000000000..f0505b7ba7f329ad57ffafa8f90a24204f002d3c
>> --- /dev/null
>> +++ b/drivers/media/i2c/og0va1b.c
>> @@ -0,0 +1,867 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * OmniVision OG0VA1B Camera Sensor Driver
>> + *
>> + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries
>> + */
>> +
>> +#include <linux/clk.h>
>> +#include <linux/delay.h>
>> +#include <linux/gpio/consumer.h>
>> +#include <linux/i2c.h>
>> +#include <linux/module.h>
>> +#include <linux/of.h>
>> +#include <linux/pm_runtime.h>
>> +#include <linux/regulator/consumer.h>
>> +
>> +#include <media/v4l2-cci.h>
>> +#include <media/v4l2-ctrls.h>
>> +#include <media/v4l2-device.h>
>> +#include <media/v4l2-fwnode.h>
>> +#include <media/v4l2-subdev.h>
>> +
>> +#define OG0VA1B_REG_CHIP_ID        CCI_REG16(0x300a)
>> +#define OG0VA1B_CHIP_ID            0xC756
> 
> This is the same chip id as of the OG0VE1B sensor device.
> 
> What's the difference between these two sensors, and do you find it 
> possible
> to add support of OG0VA1B sensor/modes into OG0VE1B sensor driver? Or is
> it just the same device?
> 
> Hardware specifics described in dt changes also does not show a difference
> in comparison to ovti,og0ve1b.yaml.
> 


Hi Vladimir,


Both OG0VE1B and OG0VA1B belong to the same OmniVision VGA monochrome 
sensor family. They share the same resolution, power rails, reset GPIO 
behavior and power management framework.

But they are different:
OG0VE1B:  8-bit RAW, lower FPS
OG0VA1B: 10-bit RAW, higher FPS
and some registers are not same.


The DT bindings can be reused, but would it be more appropriate for the 
driver to remain independent?


Thanks,
Wenmeng




^ permalink raw reply

* Re: [PATCH 2/2] pwm: add Axiado AX3000 PWM driver
From: sashiko-bot @ 2026-06-18 12:33 UTC (permalink / raw)
  To: Petar Stepanovic; +Cc: robh, devicetree, conor+dt
In-Reply-To: <20260618-axiado-ax3000-pwm-v1-2-c9797a909414@axiado.com>

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Unclocked MMIO write leading to bus error on driver unbind or probe failure.
- [High] PWM hardware fails to re-enable when transitioning from a 0% duty cycle back to a positive duty cycle.
--

commit a497ac78be310d772f657a548c4a134269cc8d5a
Author: Petar Stepanovic <pstepanovic@axiado.com>

pwm: add Axiado AX3000 PWM driver
    
The Axiado AX3000 and AX3005 SoCs include PWM controllers that can be
used to generate configurable PWM output signals.
    
Add a PWM driver with support for configuring period, duty cycle, and
enable state through the Linux PWM framework.

> diff --git a/drivers/pwm/pwm-axiado.c b/drivers/pwm/pwm-axiado.c
> new file mode 100644
> index 0000000000000..db197886c5c46
> --- /dev/null
> +++ b/drivers/pwm/pwm-axiado.c

[ ... ]

> +static int axiado_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
> +			    const struct pwm_state *state)
> +{
> +	struct axiado_pwm_chip *axpwm = pwmchip_get_drvdata(chip);
> +	int ret;
> +
> +	if (state->polarity != PWM_POLARITY_NORMAL)
> +		return -EINVAL;
> +
> +	if (!state->enabled || !state->duty_cycle) {
> +		if (pwm->state.enabled)
> +			writel(AX_PWM_CTRL_DISABLE, axpwm->base + AX_PWM_CNTRL_REG);
> +
> +		return 0;
> +	}
> +
> +	ret = axiado_pwm_config(chip, pwm, state->duty_cycle, state->period);
> +	if (ret)
> +		return ret;
> +
> +	if (!pwm->state.enabled)
> +		writel(AX_PWM_CTRL_ENABLE, axpwm->base + AX_PWM_CNTRL_REG);

[Severity: High]
Will this logic fail to re-enable the hardware if transitioning from a 0%
duty cycle back to a positive duty cycle?

When the user requests a 0% duty cycle, the hardware is turned off by
writing AX_PWM_CTRL_DISABLE, but the PWM framework maintains
pwm->state.enabled = true.

If a positive duty cycle is then requested, the driver relies on checking
!pwm->state.enabled to re-enable the hardware. Since the cached state is
still true from the 0% duty cycle request, the check evaluates to false
and the AX_PWM_CTRL_ENABLE write is skipped, leaving the physical hardware
disabled.

[ ... ]

> +static void axiado_pwm_disable(void *data)
> +{
> +	struct axiado_pwm_chip *axpwm = data;
> +
> +	writel(AX_PWM_CTRL_DISABLE, axpwm->base + AX_PWM_CNTRL_REG);
> +}
> +
> +static int axiado_pwm_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct axiado_pwm_chip *axpwm;
> +	struct pwm_chip *chip;
> +	int ret;
> +
> +	chip = devm_pwmchip_alloc(dev, AX_PWM_NUM, sizeof(*axpwm));
> +	if (IS_ERR(chip))
> +		return PTR_ERR(chip);
> +
> +	axpwm = pwmchip_get_drvdata(chip);
> +
> +	axpwm->base = devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(axpwm->base))
> +		return dev_err_probe(dev, PTR_ERR(axpwm->base),
> +				     "failed to map registers\n");
> +
> +	ret = devm_add_action_or_reset(dev, axiado_pwm_disable, axpwm);
> +	if (ret)
> +		return ret;
> +
> +
> +	axpwm->clk = devm_clk_get_enabled(dev, "pwm");

[Severity: High]
Can this cause an unclocked MMIO write during driver unbind or probe failure?

The devres cleanup action axiado_pwm_disable() is registered via
devm_add_action_or_reset() before the hardware clock is acquired and
enabled via devm_clk_get_enabled().

Because devres executes actions in LIFO order, on driver unbind the
clock will be disabled before axiado_pwm_disable() is called. Similarly,
if devm_clk_get_enabled() fails, axiado_pwm_disable() will execute while
the clock is off.

Since axiado_pwm_disable() writes to the AX_PWM_CNTRL_REG MMIO register,
accessing it without the clock active could cause a bus error.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260618-axiado-ax3000-pwm-v1-0-c9797a909414@axiado.com?part=2

^ permalink raw reply

* Re: [PATCH v2 2/2] drm/panel: Add driver for Raydium RM69220 DDIC
From: Neil Armstrong @ 2026-06-18 12:30 UTC (permalink / raw)
  To: Alexander Koskovich, Jessica Zhang, David Airlie, Simona Vetter,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley
  Cc: dri-devel, devicetree, linux-kernel
In-Reply-To: <20260605-asteroids-panel-support-v2-2-44b96c6eca91@pm.me>

On 6/5/26 20:27, Alexander Koskovich wrote:
> Add a panel driver for Raydium RM69220 DDIC based displays such as the
> BOE BF068MWM-TD0 found in the Nothing Phone (3a).
> 
> Signed-off-by: Alexander Koskovich <akoskovich@pm.me>
> ---
>   MAINTAINERS                                   |   1 +
>   drivers/gpu/drm/panel/Kconfig                 |  15 +
>   drivers/gpu/drm/panel/Makefile                |   1 +
>   drivers/gpu/drm/panel/panel-raydium-rm69220.c | 435 ++++++++++++++++++++++++++
>   4 files changed, 452 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index ca2417d9d238..367f8a42e485 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -8358,6 +8358,7 @@ DRM DRIVER FOR RAYDIUM RM69220 PANELS
>   M:	Alexander Koskovich <akoskovich@pm.me>
>   S:	Maintained
>   F:	Documentation/devicetree/bindings/display/panel/raydium,rm69220.yaml
> +F:	drivers/gpu/drm/panel/panel-raydium-rm69220.c
>   
>   DRM DRIVER FOR SAMSUNG DB7430 PANELS
>   M:	Linus Walleij <linusw@kernel.org>
> diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
> index 7450b27622a2..142c91202b8f 100644
> --- a/drivers/gpu/drm/panel/Kconfig
> +++ b/drivers/gpu/drm/panel/Kconfig
> @@ -769,6 +769,21 @@ config DRM_PANEL_RAYDIUM_RM692E5
>   	  Say Y here if you want to enable support for Raydium RM692E5-based
>   	  display panels, such as the one found in the Fairphone 5 smartphone.
>   
> +config DRM_PANEL_RAYDIUM_RM69220
> +	tristate "Raydium RM69220-based DSI panel"
> +	depends on OF
> +	depends on DRM_MIPI_DSI
> +	depends on BACKLIGHT_CLASS_DEVICE
> +	select DRM_DISPLAY_DSC_HELPER
> +	select DRM_DISPLAY_HELPER
> +	help
> +	  Say Y here if you want to enable support for Raydium RM69220-based
> +	  display panels, such as the BOE BF068MWM-TD0 which can be found in
> +	  the Nothing Phone (3a).
> +
> +	  To compile this driver as a module, choose M here: the module
> +	  will be called panel-raydium-rm69220.
> +
>   config DRM_PANEL_RAYDIUM_RM69380
>   	tristate "Raydium RM69380-based DSI panel"
>   	depends on GPIOLIB
> diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
> index c2c5cf817116..4c23ac013f12 100644
> --- a/drivers/gpu/drm/panel/Makefile
> +++ b/drivers/gpu/drm/panel/Makefile
> @@ -76,6 +76,7 @@ obj-$(CONFIG_DRM_PANEL_RAYDIUM_RM67191) += panel-raydium-rm67191.o
>   obj-$(CONFIG_DRM_PANEL_RAYDIUM_RM67200) += panel-raydium-rm67200.o
>   obj-$(CONFIG_DRM_PANEL_RAYDIUM_RM68200) += panel-raydium-rm68200.o
>   obj-$(CONFIG_DRM_PANEL_RAYDIUM_RM692E5) += panel-raydium-rm692e5.o
> +obj-$(CONFIG_DRM_PANEL_RAYDIUM_RM69220) += panel-raydium-rm69220.o
>   obj-$(CONFIG_DRM_PANEL_RAYDIUM_RM69380) += panel-raydium-rm69380.o
>   obj-$(CONFIG_DRM_PANEL_RENESAS_R61307) += panel-renesas-r61307.o
>   obj-$(CONFIG_DRM_PANEL_RENESAS_R69328) += panel-renesas-r69328.o
> diff --git a/drivers/gpu/drm/panel/panel-raydium-rm69220.c b/drivers/gpu/drm/panel/panel-raydium-rm69220.c
> new file mode 100644
> index 000000000000..9f5dd51b4d5e
> --- /dev/null
> +++ b/drivers/gpu/drm/panel/panel-raydium-rm69220.c
> @@ -0,0 +1,435 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Generated with linux-mdss-dsi-panel-driver-generator from vendor device tree.
> + * Copyright (c) 2026 Alexander Koskovich <akoskovich@pm.me>
> + */
> +
> +#include <linux/backlight.h>
> +#include <linux/delay.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/module.h>
> +#include <linux/regulator/consumer.h>
> +
> +#include <video/mipi_display.h>
> +
> +#include <drm/display/drm_dsc.h>
> +#include <drm/display/drm_dsc_helper.h>
> +#include <drm/drm_mipi_dsi.h>
> +#include <drm/drm_modes.h>
> +#include <drm/drm_panel.h>
> +#include <drm/drm_probe_helper.h>
> +
> +struct raydium_rm69220 {
> +	struct drm_panel panel;
> +	struct mipi_dsi_device *dsi;
> +	struct drm_dsc_config dsc;
> +	struct regulator_bulk_data *supplies;
> +	struct gpio_desc *reset_gpio;
> +};
> +
> +static const struct regulator_bulk_data raydium_rm69220_supplies[] = {
> +	{ .supply = "vddio" },
> +	{ .supply = "dvdd" },
> +	{ .supply = "vci" },
> +};
> +
> +static inline
> +struct raydium_rm69220 *to_raydium_rm69220(struct drm_panel *panel)
> +{
> +	return container_of(panel, struct raydium_rm69220, panel);
> +}
> +
> +static void raydium_rm69220_reset(struct raydium_rm69220 *ctx)
> +{
> +	gpiod_set_value_cansleep(ctx->reset_gpio, 0);
> +	usleep_range(1000, 2000);
> +	gpiod_set_value_cansleep(ctx->reset_gpio, 1);
> +	usleep_range(1000, 2000);
> +	gpiod_set_value_cansleep(ctx->reset_gpio, 0);
> +	msleep(32);
> +}
> +
> +static int raydium_rm69220_on(struct raydium_rm69220 *ctx)
> +{
> +	struct mipi_dsi_multi_context dsi_ctx = { .dsi = ctx->dsi };
> +
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xfe, 0x76);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x9a, 0x10);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x9b, 0x00);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xfe, 0x77);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x9a, 0x10);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x9b, 0x00);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xfe, 0x78);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x9a, 0x10);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x9b, 0x00);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xfe, 0x79);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x9a, 0xf0);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x9b, 0x00);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xfe, 0x74);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x1a, 0xe0);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x1b, 0x00);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xfe, 0x40);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xa9, 0x68);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xab, 0x22);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xfe, 0x40);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xc0, 0xa6);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xbf, 0x87);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xfe, 0x82);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x00, 0x90);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xfe, 0xd4);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x61, 0x08);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xa2, 0x04);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xfe, 0x00);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xfa, 0x01);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xfe, 0xd2);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x97, 0x08);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x36, 0x11);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x39, 0xab);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x3a, 0x30);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x3b, 0x80);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x3d, 0x09);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x3f, 0x58);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x40, 0x04);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x41, 0x38);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x42, 0x00);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x43, 0x0d);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x44, 0x02);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x45, 0x1c);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x46, 0x02);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x47, 0x1c);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x48, 0x02);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x49, 0x00);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x4a, 0x02);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x4b, 0x0e);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x4d, 0x20);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x4e, 0x01);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x4f, 0x39);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x50, 0x00);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x51, 0x07);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x53, 0x0c);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x54, 0x08);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x55, 0x00);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x56, 0x07);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x58, 0xd3);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x59, 0x18);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x5a, 0x00);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x5b, 0x10);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x5c, 0xf0);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x5d, 0x07);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x5e, 0x10);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x5f, 0x20);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x60, 0x00);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x61, 0x06);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x62, 0x0f);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x63, 0x0f);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x64, 0x33);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x65, 0x0e);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x66, 0x1c);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x67, 0x2a);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x68, 0x38);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x69, 0x46);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x6a, 0x54);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x6b, 0x62);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x6c, 0x69);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x6d, 0x70);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x6e, 0x77);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x6f, 0x79);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x70, 0x7b);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x71, 0x7d);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x72, 0x7e);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x73, 0x01);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x74, 0x02);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x75, 0x22);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x76, 0x00);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x77, 0x2a);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x78, 0x40);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x79, 0x2a);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x7a, 0xbe);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x7b, 0x3a);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x7c, 0xfc);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x7d, 0x3a);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x7e, 0xfa);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x7f, 0x3a);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x80, 0xf8);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x81, 0x3b);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x82, 0x38);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x83, 0x3b);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x84, 0x78);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x85, 0x3b);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x86, 0xb6);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x87, 0x4b);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x88, 0xf6);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x89, 0x4c);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x8a, 0x34);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x8b, 0x4c);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x8c, 0x74);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x8d, 0x5c);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x8e, 0x74);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x8f, 0x8c);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x90, 0xf4);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x91, 0x00);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x92, 0x00);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x93, 0x00);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x94, 0x00);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x95, 0x00);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x96, 0x00);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xfe, 0xa0);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x66, 0x06);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xfe, 0x49);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x87, 0x00);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x3f, 0x00);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xfe, 0xa0);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x04, 0x07);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x4b, 0x40);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x4d, 0x40);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xfe, 0xa1);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x58, 0x67);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x75, 0xa7);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xfe, 0x00);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x2f, 0x00);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xfe, 0x00);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xfa, 0x01);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xc1, 0x03);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xc2, 0x03);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x35, 0x00);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x51, 0x0d, 0xbb);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xfe, 0x00);
> +	mipi_dsi_dcs_exit_sleep_mode_multi(&dsi_ctx);
> +	mipi_dsi_msleep(&dsi_ctx, 80);
> +	mipi_dsi_dcs_set_display_on_multi(&dsi_ctx);
> +	mipi_dsi_usleep_range(&dsi_ctx, 10000, 11000);
> +
> +	return dsi_ctx.accum_err;
> +}
> +
> +static int raydium_rm69220_off(struct raydium_rm69220 *ctx)
> +{
> +	struct mipi_dsi_multi_context dsi_ctx = { .dsi = ctx->dsi };
> +
> +	mipi_dsi_dcs_set_display_off_multi(&dsi_ctx);
> +	mipi_dsi_msleep(&dsi_ctx, 100);
> +	mipi_dsi_dcs_enter_sleep_mode_multi(&dsi_ctx);
> +	mipi_dsi_msleep(&dsi_ctx, 100);
> +
> +	return dsi_ctx.accum_err;
> +}
> +
> +static int raydium_rm69220_prepare(struct drm_panel *panel)
> +{
> +	struct raydium_rm69220 *ctx = to_raydium_rm69220(panel);
> +	struct device *dev = &ctx->dsi->dev;
> +	struct drm_dsc_picture_parameter_set pps;
> +	int ret;
> +
> +	ret = regulator_bulk_enable(ARRAY_SIZE(raydium_rm69220_supplies), ctx->supplies);
> +	if (ret < 0) {
> +		dev_err(dev, "Failed to enable regulators: %d\n", ret);
> +		return ret;
> +	}
> +
> +	raydium_rm69220_reset(ctx);
> +
> +	ret = raydium_rm69220_on(ctx);
> +	if (ret < 0) {
> +		dev_err(dev, "Failed to initialize panel: %d\n", ret);
> +		goto err;
> +	}
> +
> +	drm_dsc_pps_payload_pack(&pps, &ctx->dsc);
> +
> +	ret = mipi_dsi_picture_parameter_set(ctx->dsi, &pps);
> +	if (ret < 0) {
> +		dev_err(dev, "failed to transmit PPS: %d\n", ret);
> +		goto err;
> +	}
> +
> +	ret = mipi_dsi_compression_mode(ctx->dsi, true);
> +	if (ret < 0) {
> +		dev_err(dev, "failed to enable compression mode: %d\n", ret);
> +		goto err;
> +	}
> +
> +	msleep(28);
> +
> +	return 0;
> +
> +err:
> +	gpiod_set_value_cansleep(ctx->reset_gpio, 1);
> +	regulator_bulk_disable(ARRAY_SIZE(raydium_rm69220_supplies), ctx->supplies);
> +	return ret;
> +}
> +
> +static int raydium_rm69220_unprepare(struct drm_panel *panel)
> +{
> +	struct raydium_rm69220 *ctx = to_raydium_rm69220(panel);
> +	struct device *dev = &ctx->dsi->dev;
> +	int ret;
> +
> +	ret = raydium_rm69220_off(ctx);
> +	if (ret < 0)
> +		dev_err(dev, "Failed to un-initialize panel: %d\n", ret);
> +
> +	gpiod_set_value_cansleep(ctx->reset_gpio, 1);
> +	regulator_bulk_disable(ARRAY_SIZE(raydium_rm69220_supplies), ctx->supplies);
> +
> +	return 0;
> +}
> +
> +static const struct drm_display_mode raydium_rm69220_mode = {
> +	.clock = (1080 + 36 + 4 + 36) * (2392 + 72 + 4 + 52) * 120 / 1000,
> +	.hdisplay = 1080,
> +	.hsync_start = 1080 + 36,
> +	.hsync_end = 1080 + 36 + 4,
> +	.htotal = 1080 + 36 + 4 + 36,
> +	.vdisplay = 2392,
> +	.vsync_start = 2392 + 72,
> +	.vsync_end = 2392 + 72 + 4,
> +	.vtotal = 2392 + 72 + 4 + 52,
> +	.width_mm = 71,
> +	.height_mm = 157,
> +	.type = DRM_MODE_TYPE_DRIVER,
> +};
> +
> +static int raydium_rm69220_get_modes(struct drm_panel *panel,
> +					    struct drm_connector *connector)
> +{
> +	return drm_connector_helper_get_modes_fixed(connector, &raydium_rm69220_mode);
> +}
> +
> +static const struct drm_panel_funcs raydium_rm69220_panel_funcs = {
> +	.prepare = raydium_rm69220_prepare,
> +	.unprepare = raydium_rm69220_unprepare,
> +	.get_modes = raydium_rm69220_get_modes,
> +};
> +
> +static int raydium_rm69220_bl_update_status(struct backlight_device *bl)
> +{
> +	struct mipi_dsi_device *dsi = bl_get_data(bl);
> +	u16 brightness = backlight_get_brightness(bl);
> +	int ret;
> +
> +	dsi->mode_flags &= ~MIPI_DSI_MODE_LPM;
> +
> +	ret = mipi_dsi_dcs_set_display_brightness_large(dsi, brightness);
> +	if (ret < 0)
> +		return ret;
> +
> +	dsi->mode_flags |= MIPI_DSI_MODE_LPM;
> +
> +	return 0;
> +}
> +
> +static const struct backlight_ops raydium_rm69220_bl_ops = {
> +	.update_status = raydium_rm69220_bl_update_status,
> +};
> +
> +static struct backlight_device *
> +raydium_rm69220_create_backlight(struct mipi_dsi_device *dsi)
> +{
> +	struct device *dev = &dsi->dev;
> +	const struct backlight_properties props = {
> +		.type = BACKLIGHT_RAW,
> +		.brightness = 2946,
> +		.max_brightness = 3442, /* 4095 is HBM max */
> +	};
> +
> +	return devm_backlight_device_register(dev, dev_name(dev), dev, dsi,
> +					      &raydium_rm69220_bl_ops, &props);
> +}
> +
> +static int raydium_rm69220_probe(struct mipi_dsi_device *dsi)
> +{
> +	struct device *dev = &dsi->dev;
> +	struct raydium_rm69220 *ctx;
> +	int ret;
> +
> +	ctx = devm_drm_panel_alloc(dev, struct raydium_rm69220, panel,
> +				   &raydium_rm69220_panel_funcs,
> +				   DRM_MODE_CONNECTOR_DSI);
> +	if (IS_ERR(ctx))
> +		return PTR_ERR(ctx);
> +
> +	ret = devm_regulator_bulk_get_const(dev,
> +					    ARRAY_SIZE(raydium_rm69220_supplies),
> +					    raydium_rm69220_supplies,
> +					    &ctx->supplies);
> +	if (ret < 0)
> +		return ret;
> +
> +	ctx->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
> +	if (IS_ERR(ctx->reset_gpio))
> +		return dev_err_probe(dev, PTR_ERR(ctx->reset_gpio),
> +				     "Failed to get reset-gpios\n");
> +
> +	ctx->dsi = dsi;
> +	mipi_dsi_set_drvdata(dsi, ctx);
> +
> +	dsi->lanes = 4;
> +	dsi->format = MIPI_DSI_FMT_RGB101010;
> +	dsi->mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_NO_EOT_PACKET |
> +			  MIPI_DSI_CLOCK_NON_CONTINUOUS | MIPI_DSI_MODE_LPM |
> +			  MIPI_DSI_MODE_DSC_ALL_SLICES_IN_PKT;
> +
> +	ctx->panel.prepare_prev_first = true;
> +
> +	ctx->panel.backlight = raydium_rm69220_create_backlight(dsi);
> +	if (IS_ERR(ctx->panel.backlight))
> +		return dev_err_probe(dev, PTR_ERR(ctx->panel.backlight),
> +				     "Failed to create backlight\n");
> +
> +	drm_panel_add(&ctx->panel);

Use devm_drm_panel_add()

> +
> +	/* This panel only supports DSC; unconditionally enable it */
> +	dsi->dsc = &ctx->dsc;
> +
> +	ctx->dsc.dsc_version_major = 1;
> +	ctx->dsc.dsc_version_minor = 1;
> +	ctx->dsc.slice_height = 13;
> +	ctx->dsc.slice_width = 540;
> +
> +	ctx->dsc.slice_count = 1080 / ctx->dsc.slice_width;
> +	ctx->dsc.bits_per_component = 10;
> +	ctx->dsc.bits_per_pixel = 8 << 4; /* 4 fractional bits */
> +	ctx->dsc.block_pred_enable = true;
> +
> +	ret = mipi_dsi_attach(dsi);

Switch to:
	return devm_mipi_dsi_attach(dev, dsi);

> +	if (ret < 0) {
> +		drm_panel_remove(&ctx->panel);
> +		return dev_err_probe(dev, ret, "Failed to attach to DSI host\n");
> +	}
> +
> +	return 0;
> +}
> +
> +static void raydium_rm69220_remove(struct mipi_dsi_device *dsi)
> +{
> +	struct raydium_rm69220 *ctx = mipi_dsi_get_drvdata(dsi);
> +	int ret;
> +
> +	ret = mipi_dsi_detach(dsi);
> +	if (ret < 0)
> +		dev_err(&dsi->dev, "Failed to detach from DSI host: %d\n", ret);
> +
> +	drm_panel_remove(&ctx->panel);
> +}

And drop the remove callback

Thanks,
Neil

> +
> +static const struct of_device_id raydium_rm69220_of_match[] = {
> +	{ .compatible = "boe,bf068mwm-td0" },
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, raydium_rm69220_of_match);
> +
> +static struct mipi_dsi_driver raydium_rm69220_driver = {
> +	.probe = raydium_rm69220_probe,
> +	.remove = raydium_rm69220_remove,
> +	.driver = {
> +		.name = "panel-raydium-rm69220",
> +		.of_match_table = raydium_rm69220_of_match,
> +	},
> +};
> +module_mipi_dsi_driver(raydium_rm69220_driver);
> +
> +MODULE_AUTHOR("Alexander Koskovich <akoskovich@pm.me>");
> +MODULE_DESCRIPTION("DRM driver for RM69220-equipped DSI panels");
> +MODULE_LICENSE("GPL");
> 


^ permalink raw reply

* Re: [PATCH v7 2/2] drm: panel: add support for the Renesas R63419 based dual-DSI video mode Display Panels
From: Neil Armstrong @ 2026-06-18 12:30 UTC (permalink / raw)
  To: Dmitry Baryshkov, Jessica Zhang, Douglas Anderson
  Cc: dri-devel, devicetree, linux-kernel, linux-renesas-soc, KancyJoe,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
	Simona Vetter, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Geert Uytterhoeven, Magnus Damm
In-Reply-To: <20260605-topic-sm8650-ayaneo-pocket-s2-r63419-v7-2-b84b6da84293@linaro.org>

Hi Dmitry, Douglas, Jessica,

On 6/5/26 16:50, Neil Armstrong wrote:
> From: KancyJoe <kancy2333@outlook.com>
> 
> Implement support for the Renesas 63419 based dual-DSI video mode
> Display Panels found in the Ayaneo gaming handled devices.
> 
> Signed-off-by: KancyJoe <kancy2333@outlook.com>
> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
> ---
>   drivers/gpu/drm/panel/Kconfig                |  12 +
>   drivers/gpu/drm/panel/Makefile               |   1 +
>   drivers/gpu/drm/panel/panel-renesas-r63419.c | 366 +++++++++++++++++++++++++++
>   3 files changed, 379 insertions(+)
> 
<snip>

Could one of you get a look on this patch so I can reduce my backlog ?

Jessica, do you still be marked as DRM Panel reviewer ?

Thanks,
Neil

^ permalink raw reply

* Re: [PATCH 1/2] dt-bindings: pwm: add Axiado AX3000 PWM
From: sashiko-bot @ 2026-06-18 12:29 UTC (permalink / raw)
  To: Petar Stepanovic; +Cc: robh, conor+dt, devicetree
In-Reply-To: <20260618-axiado-ax3000-pwm-v1-1-c9797a909414@axiado.com>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The `#pwm-cells` property is missing from the `required` list in the device tree binding schema.
--

commit ecac4ef8f05dd912d17935a0faabe9c81c3d921c
Author: Petar Stepanovic <pstepanovic@axiado.com>

dt-bindings: pwm: add Axiado AX3000 PWM

This commit introduces the device tree binding schema and maintainer entry
for the Axiado AX3000 and AX3005 PWM controllers.

> diff --git a/Documentation/devicetree/bindings/pwm/axiado,ax3000-pwm.yaml b/Documentation/devicetree/bindings/pwm/axiado,ax3000-pwm.yaml
> new file mode 100644
> index 0000000000000..8e5fee8c499d5
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pwm/axiado,ax3000-pwm.yaml

[ ... ]

> +  "#pwm-cells":
> +    const: 2
> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks

[Severity: Medium]
Should "#pwm-cells" be included in this required list?

According to the device tree bindings subsystem guidelines, when a hardware
component provides PWM capabilities, the corresponding cell-count property
("#pwm-cells") must be explicitly included in the required list.

Omitting it allows incomplete device tree nodes missing the "#pwm-cells"
property to silently pass schema validation. This defeats the purpose of
the schema validation and can lead to runtime failures if dependent drivers
attempt to use an improperly configured node.

> +
> +additionalProperties: false

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260618-axiado-ax3000-pwm-v1-0-c9797a909414@axiado.com?part=1

^ permalink raw reply

* Re: [PATCH v3 2/3] media: i2c: add os02g10 image sensor driver
From: Tarang Raval @ 2026-06-18 12:27 UTC (permalink / raw)
  To: Sakari Ailus, Elgin Perumbilly
  Cc: laurent.pinchart@ideasonboard.com, Mauro Carvalho Chehab,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Hans Verkuil,
	Hans de Goede, Vladimir Zapolskiy, Mehdi Djait, Sylvain Petinot,
	Benjamin Mugnier, Bryan O'Donoghue, Himanshu Bhavani,
	Heimir Thor Sverrisson, Jingjing Xiong, Svyatoslav Ryhel,
	linux-media@vger.kernel.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <ajPZ_mURqAFbk59S@kekkonen.localdomain>

Hi Sakari, Elgin

> > +static int os02g10_power_on(struct device *dev)
> > +{
> > +     struct v4l2_subdev *sd = dev_get_drvdata(dev);
> > +     struct os02g10 *os02g10 = to_os02g10(sd);
> > +     int ret;
> > +
> > +     ret = regulator_bulk_enable(ARRAY_SIZE(os02g10_supply_name),
> > +                                 os02g10->supplies);
> > +     if (ret) {
> > +             dev_err(os02g10->dev, "failed to enable regulators\n");
> > +             return ret;
> > +     }
> > +
> > +     /* T4: delay from DOVDD stable to MCLK on */
> > +     fsleep(5 * USEC_PER_MSEC);
>
> Does the sensor really require this? Typically no delays are required
> before lifting xshutdown.

The 5 ms delay for T4 (DOVDD stable to ECLK on) is required by the
datasheet.

However, T3 and T4 are independent timing requirements, not sequential
ones. The current implementation introduces an unnecessary extra 5 ms
delay before deasserting XSHUTDN.

The comment could also be updated to:
/* Wait for T3/T4 timing requirements after supplies become stable */


Power-up Sequence:

DOVDD    ________/─────────────────────────────────────────────────────
                 |
                 |<------ T4 = 5 ms ------>|
                 |                         |
                 |                         +------ ECLK ON

AVDD     ________________/─────────────────────────────────────────────
                      ^
                      |
                    T1 >= 0 ms

DVDD     ________________________/─────────────────────────────────────
                               |
                               |<------ T3 = 5 ms ------>|
                               |                         |
                               |                         +------ XSHUTDN Release

XSHUTDN  ____________________________________/─────────────────────────

ECLK     __________________________________________/\/\/\/\/\/\/\/\/───

SCCB     ____________________________________________________XXXXXXXXXX
                                                 ^
                                                 |
                                 T5 = 5 ms ------+

> > +
> > +     ret = clk_prepare_enable(os02g10->xclk);
> > +     if (ret) {
> > +             dev_err(os02g10->dev, "failed to enable clock\n");
> > +             goto err_regulator_off;
> > +     }
> > +
> > +     /* T3: delay from DVDD stable to sensor power up stable */
> > +     fsleep(5 * USEC_PER_MSEC);
>
> The supplies were enabled before the clock so right now there's already 5
> ms delay here. Consider with the above comment.
>
> > +
> > +     gpiod_set_value_cansleep(os02g10->reset_gpio, 0);
> > +
> > +     /* T5: delay from sensor power up stable to SCCB initialization */
> > +     fsleep(5 * USEC_PER_MSEC);
> > +
> > +     return 0;
> > +
> > +err_regulator_off:
> > +     regulator_bulk_disable(ARRAY_SIZE(os02g10_supply_name), os02g10->supplies);
>
> A newline here would be nice.
>
> > +     return ret;
> > +}

Best Regards,
Tarang

^ permalink raw reply

* [PATCH 0/2] pwm: add Axiado AX3000 PWM support
From: Petar Stepanovic @ 2026-06-18 12:26 UTC (permalink / raw)
  To: Akhila Kavi, Prasad Bolisetty, Uwe Kleine-König, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Harshit Shah
  Cc: linux-pwm, devicetree, linux-arm-kernel, linux-kernel,
	Petar Stepanovic

This series adds support for the PWM controller found on Axiado
AX3000 and AX3005 SoCs.

A new driver is needed because this PWM controller is a SoC-specific
hardware block used on Axiado SoCs. It has its own register layout,
enable control, period configuration, duty-cycle configuration, and
does not match any existing upstream PWM driver.

The controller provides configurable PWM output signals. The driver
exposes the controller through the Linux PWM framework and supports
period and duty-cycle configuration.

The driver converts the requested period and duty cycle from nanoseconds
to hardware clock cycles based on the input clock rate.

The datasheet is not publicly available. Public high-level product
information is available at:

https://axiado.com/products/#AX3080

The register definitions and programming sequence used by this driver
are based on Axiado internal SoC documentation.

Signed-off-by: Petar Stepanovic <pstepanovic@axiado.com>
---
Petar Stepanovic (2):
      dt-bindings: pwm: add Axiado AX3000 PWM
      pwm: add Axiado AX3000 PWM driver

 .../devicetree/bindings/pwm/axiado,ax3000-pwm.yaml |  52 ++++++
 MAINTAINERS                                        |   9 +
 drivers/pwm/Kconfig                                |  11 ++
 drivers/pwm/Makefile                               |   1 +
 drivers/pwm/pwm-axiado.c                           | 193 +++++++++++++++++++++
 5 files changed, 266 insertions(+)
---
base-commit: 1d5dcaa3bd65f2e8c9baa14a393d3a2dc5db7524
change-id: 20260518-axiado-ax3000-pwm-cd7c346849f1

Best regards,
-- 
Petar Stepanovic <pstepanovic@axiado.com>


^ permalink raw reply

* [PATCH 2/2] pwm: add Axiado AX3000 PWM driver
From: Petar Stepanovic @ 2026-06-18 12:26 UTC (permalink / raw)
  To: Akhila Kavi, Prasad Bolisetty, Uwe Kleine-König, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Harshit Shah
  Cc: linux-pwm, devicetree, linux-arm-kernel, linux-kernel,
	Petar Stepanovic
In-Reply-To: <20260618-axiado-ax3000-pwm-v1-0-c9797a909414@axiado.com>

The Axiado AX3000 and AX3005 SoCs include PWM controllers that can be
used to generate configurable PWM output signals.

Add a PWM driver with support for configuring period, duty cycle, and
enable state through the Linux PWM framework.

Signed-off-by: Petar Stepanovic <pstepanovic@axiado.com>
---
 MAINTAINERS              |   1 +
 drivers/pwm/Kconfig      |  11 +++
 drivers/pwm/Makefile     |   1 +
 drivers/pwm/pwm-axiado.c | 193 +++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 206 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 394c4a3527e8..db93fc235c32 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4319,6 +4319,7 @@ M:	Prasad Bolisetty <pbolisetty@axiado.com>
 L:	linux-pwm@vger.kernel.org
 S:	Maintained
 F:	Documentation/devicetree/bindings/pwm/axiado,ax3000-pwm.yaml
+F:	drivers/pwm/pwm-axiado.c
 
 AXIS ARTPEC ARM64 SoC SUPPORT
 M:	Jesper Nilsson <jesper.nilsson@axis.com>
diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
index 6f3147518376..76f6c04b0e23 100644
--- a/drivers/pwm/Kconfig
+++ b/drivers/pwm/Kconfig
@@ -129,6 +129,17 @@ config PWM_ATMEL_TCB
 	  To compile this driver as a module, choose M here: the module
 	  will be called pwm-atmel-tcb.
 
+config PWM_AXIADO
+	tristate "Axiado PWM support"
+	depends on ARCH_AXIADO || COMPILE_TEST
+	depends on HAS_IOMEM
+	help
+	  PWM framework driver for the PWM controller found on Axiado
+	  AX3000 and AX3005 SoCs.
+
+	  To compile this driver as a module, choose M here: the module
+	  will be called pwm-axiado.
+
 config PWM_AXI_PWMGEN
 	tristate "Analog Devices AXI PWM generator"
 	depends on MICROBLAZE || NIOS2 || ARCH_ZYNQ || ARCH_ZYNQMP || ARCH_INTEL_SOCFPGA || COMPILE_TEST
diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile
index 0dc0d2b69025..4466a29e780a 100644
--- a/drivers/pwm/Makefile
+++ b/drivers/pwm/Makefile
@@ -8,6 +8,7 @@ obj-$(CONFIG_PWM_ARGON_FAN_HAT)	+= pwm-argon-fan-hat.o
 obj-$(CONFIG_PWM_ATMEL)		+= pwm-atmel.o
 obj-$(CONFIG_PWM_ATMEL_HLCDC_PWM)	+= pwm-atmel-hlcdc.o
 obj-$(CONFIG_PWM_ATMEL_TCB)	+= pwm-atmel-tcb.o
+obj-$(CONFIG_PWM_AXIADO)	+= pwm-axiado.o
 obj-$(CONFIG_PWM_AXI_PWMGEN)	+= pwm-axi-pwmgen.o
 obj-$(CONFIG_PWM_BCM2835)	+= pwm-bcm2835.o
 obj-$(CONFIG_PWM_BCM_IPROC)	+= pwm-bcm-iproc.o
diff --git a/drivers/pwm/pwm-axiado.c b/drivers/pwm/pwm-axiado.c
new file mode 100644
index 000000000000..db197886c5c4
--- /dev/null
+++ b/drivers/pwm/pwm-axiado.c
@@ -0,0 +1,193 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2021-2026 Axiado Corporation.
+ */
+
+#include <linux/bits.h>
+#include <linux/clk.h>
+#include <linux/io.h>
+#include <linux/math64.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/pwm.h>
+
+/* Register offsets */
+#define AX_PWM_CNTRL_REG     0x0000
+#define AX_PWM_PERIOD_REG    0x0004
+#define AX_PWM_HIGH_REG      0x0008
+
+/* PWM channels */
+#define AX_PWM_NUM 1
+
+/* Period and duty cycle limits */
+#define AX_PWM_PERIOD_MIN       2
+#define AX_PWM_PERIOD_MAX       0xfffffffeU
+#define AX_PWM_DUTY_MIN         1
+#define AX_PWM_DUTY_MAX         0xfffffffdU
+
+/* Control register bits */
+#define AX_PWM_CTRL_ENABLE BIT(0)
+#define AX_PWM_CTRL_DISABLE 0x0
+
+struct axiado_pwm_chip {
+	struct clk *clk;
+	void __iomem *base;
+};
+
+static int axiado_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
+			     u64 duty_ns, u64 period_ns)
+{
+	struct axiado_pwm_chip *axpwm = pwmchip_get_drvdata(chip);
+	unsigned long rate;
+	u64 period_cycles, duty_cycles;
+
+	/*
+	 * The hardware does not support a zero period, 0% duty cycle, or
+	 * 100% duty cycle. The caller should handle 0% duty cycle by
+	 * disabling the PWM.
+	 */
+	if (!period_ns || !duty_ns || duty_ns >= period_ns)
+		return -EINVAL;
+
+	rate = clk_get_rate(axpwm->clk);
+	if (!rate)
+		return -EINVAL;
+
+	period_cycles = mul_u64_u64_div_u64(period_ns, rate, NSEC_PER_SEC);
+	if (period_cycles < AX_PWM_PERIOD_MIN ||
+	    period_cycles > AX_PWM_PERIOD_MAX)
+		return -EINVAL;
+
+	duty_cycles = mul_u64_u64_div_u64(duty_ns, rate, NSEC_PER_SEC);
+	if (duty_cycles < AX_PWM_DUTY_MIN ||
+	    duty_cycles > AX_PWM_DUTY_MAX)
+		return -EINVAL;
+
+	if (duty_cycles >= period_cycles)
+		return -EINVAL;
+
+	writel((u32)period_cycles, axpwm->base + AX_PWM_PERIOD_REG);
+	writel((u32)duty_cycles, axpwm->base + AX_PWM_HIGH_REG);
+
+	return 0;
+}
+
+static int axiado_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
+			    const struct pwm_state *state)
+{
+	struct axiado_pwm_chip *axpwm = pwmchip_get_drvdata(chip);
+	int ret;
+
+	if (state->polarity != PWM_POLARITY_NORMAL)
+		return -EINVAL;
+
+	if (!state->enabled || !state->duty_cycle) {
+		if (pwm->state.enabled)
+			writel(AX_PWM_CTRL_DISABLE, axpwm->base + AX_PWM_CNTRL_REG);
+
+		return 0;
+	}
+
+	ret = axiado_pwm_config(chip, pwm, state->duty_cycle, state->period);
+	if (ret)
+		return ret;
+
+	if (!pwm->state.enabled)
+		writel(AX_PWM_CTRL_ENABLE, axpwm->base + AX_PWM_CNTRL_REG);
+
+	return 0;
+}
+
+static int axiado_pwm_get_state(struct pwm_chip *chip, struct pwm_device *pwm,
+				struct pwm_state *state)
+{
+	struct axiado_pwm_chip *axpwm = pwmchip_get_drvdata(chip);
+	unsigned long rate;
+	u32 period_cycles;
+	u32 duty_cycles;
+	u32 ctrl;
+
+	rate = clk_get_rate(axpwm->clk);
+	if (!rate)
+		return -EINVAL;
+
+	ctrl = readl(axpwm->base + AX_PWM_CNTRL_REG);
+	period_cycles = readl(axpwm->base + AX_PWM_PERIOD_REG);
+	duty_cycles = readl(axpwm->base + AX_PWM_HIGH_REG);
+
+	state->enabled = !!(ctrl & AX_PWM_CTRL_ENABLE);
+	state->period = mul_u64_u64_div_u64(period_cycles, NSEC_PER_SEC, rate);
+	state->duty_cycle = mul_u64_u64_div_u64(duty_cycles, NSEC_PER_SEC, rate);
+	state->polarity = PWM_POLARITY_NORMAL;
+
+	return 0;
+}
+
+static const struct pwm_ops axiado_pwm_ops = {
+	.get_state = axiado_pwm_get_state,
+	.apply = axiado_pwm_apply,
+};
+
+static void axiado_pwm_disable(void *data)
+{
+	struct axiado_pwm_chip *axpwm = data;
+
+	writel(AX_PWM_CTRL_DISABLE, axpwm->base + AX_PWM_CNTRL_REG);
+}
+
+static int axiado_pwm_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct axiado_pwm_chip *axpwm;
+	struct pwm_chip *chip;
+	int ret;
+
+	chip = devm_pwmchip_alloc(dev, AX_PWM_NUM, sizeof(*axpwm));
+	if (IS_ERR(chip))
+		return PTR_ERR(chip);
+
+	axpwm = pwmchip_get_drvdata(chip);
+
+	axpwm->base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(axpwm->base))
+		return dev_err_probe(dev, PTR_ERR(axpwm->base),
+				     "failed to map registers\n");
+
+	ret = devm_add_action_or_reset(dev, axiado_pwm_disable, axpwm);
+	if (ret)
+		return ret;
+
+
+	axpwm->clk = devm_clk_get_enabled(dev, "pwm");
+	if (IS_ERR(axpwm->clk))
+		return dev_err_probe(dev, PTR_ERR(axpwm->clk),
+				     "failed to get/enable clock\n");
+
+	chip->ops = &axiado_pwm_ops;
+
+	ret = devm_pwmchip_add(dev, chip);
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to add PWM chip\n");
+
+	return 0;
+}
+
+static const struct of_device_id axiado_pwm_match[] = {
+	{ .compatible = "axiado,ax3000-pwm" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, axiado_pwm_match);
+
+static struct platform_driver axiado_pwm_driver = {
+	.driver = {
+		.name =  "axiado-pwm",
+		.of_match_table = axiado_pwm_match,
+	},
+	.probe = axiado_pwm_probe,
+};
+
+module_platform_driver(axiado_pwm_driver);
+
+MODULE_AUTHOR("Axiado Corporation");
+MODULE_DESCRIPTION("Axiado PWM driver");
+MODULE_LICENSE("GPL");

-- 
2.34.1


^ permalink raw reply related

* [PATCH 1/2] dt-bindings: pwm: add Axiado AX3000 PWM
From: Petar Stepanovic @ 2026-06-18 12:26 UTC (permalink / raw)
  To: Akhila Kavi, Prasad Bolisetty, Uwe Kleine-König, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Harshit Shah
  Cc: linux-pwm, devicetree, linux-arm-kernel, linux-kernel,
	Petar Stepanovic
In-Reply-To: <20260618-axiado-ax3000-pwm-v1-0-c9797a909414@axiado.com>

The Axiado AX3000 and AX3005 SoCs include PWM controllers that can be
used to generate configurable PWM output signals.

Signed-off-by: Petar Stepanovic <pstepanovic@axiado.com>
---
 .../devicetree/bindings/pwm/axiado,ax3000-pwm.yaml | 52 ++++++++++++++++++++++
 MAINTAINERS                                        |  8 ++++
 2 files changed, 60 insertions(+)

diff --git a/Documentation/devicetree/bindings/pwm/axiado,ax3000-pwm.yaml b/Documentation/devicetree/bindings/pwm/axiado,ax3000-pwm.yaml
new file mode 100644
index 000000000000..8e5fee8c499d
--- /dev/null
+++ b/Documentation/devicetree/bindings/pwm/axiado,ax3000-pwm.yaml
@@ -0,0 +1,52 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pwm/axiado,ax3000-pwm.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Axiado AX3000 PWM controller
+
+maintainers:
+  - Petar Stepanovic <pstepanovic@axiado.com>
+  - Akhila Kavi <akavi@axiado.com>
+  - Prasad Bolisetty <pbolisetty@axiado.com>
+
+description:
+  The Axiado PWM controller found on AX3000 and AX3005 SoCs.
+
+allOf:
+  - $ref: pwm.yaml#
+
+properties:
+  compatible:
+    const: axiado,ax3000-pwm
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+  clock-names:
+    const: pwm
+
+  "#pwm-cells":
+    const: 2
+
+required:
+  - compatible
+  - reg
+  - clocks
+
+additionalProperties: false
+
+examples:
+  - |
+    pwm@80801c00 {
+      compatible = "axiado,ax3000-pwm";
+      reg = <0x80801c00 0x1000>;
+      #pwm-cells = <2>;
+      clocks = <&clk>;
+      clock-names = "pwm";
+    };
+
diff --git a/MAINTAINERS b/MAINTAINERS
index b2040011a386..394c4a3527e8 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4312,6 +4312,14 @@ S:	Orphan
 F:	Documentation/devicetree/bindings/sound/axentia,*
 F:	sound/soc/atmel/tse850-pcm5142.c
 
+AXIADO AX3000 PWM DRIVER
+M:	Petar Stepanovic <pstepanovic@axiado.com>
+M:	Akhila Kavi <akavi@axiado.com>
+M:	Prasad Bolisetty <pbolisetty@axiado.com>
+L:	linux-pwm@vger.kernel.org
+S:	Maintained
+F:	Documentation/devicetree/bindings/pwm/axiado,ax3000-pwm.yaml
+
 AXIS ARTPEC ARM64 SoC SUPPORT
 M:	Jesper Nilsson <jesper.nilsson@axis.com>
 M:	Lars Persson <lars.persson@axis.com>

-- 
2.34.1


^ permalink raw reply related

* Re: [PATCH V5 4/6] drm/panel: anbernic-td4310: Add RG Vita Pro panel
From: neil.armstrong @ 2026-06-18 12:26 UTC (permalink / raw)
  To: Chris Morgan, linux-rockchip
  Cc: devicetree, xsf, sre, simona, airlied, tzimmermann, mripard,
	maarten.lankhorst, jesszhan0024, heiko, conor+dt, krzk+dt, robh,
	Chris Morgan
In-Reply-To: <20260604220802.119107-5-macroalpha82@gmail.com>

On 6/5/26 00:07, Chris Morgan wrote:
> From: Chris Morgan <macromorgan@hotmail.com>
> 
> The panel used by Anbernic in the RG Vita-Pro is a DSI panel based
> on the TD4310 controller IC. It measures approximately 5.5 inches
> diagonally and is 1080x1920 in resolution.
> 
> Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> ---
>   drivers/gpu/drm/panel/Kconfig                 |  10 +
>   drivers/gpu/drm/panel/Makefile                |   1 +
>   drivers/gpu/drm/panel/panel-anbernic-td4310.c | 256 ++++++++++++++++++
>   3 files changed, 267 insertions(+)
>   create mode 100644 drivers/gpu/drm/panel/panel-anbernic-td4310.c
> 
> diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
> index d592f4f4b939..61dd00297ecc 100644
> --- a/drivers/gpu/drm/panel/Kconfig
> +++ b/drivers/gpu/drm/panel/Kconfig
> @@ -17,6 +17,16 @@ config DRM_PANEL_ABT_Y030XX067A
>   	  Y030XX067A 320x480 3.0" panel as found in the YLM RG-280M, RG-300
>   	  and RG-99 handheld gaming consoles.
>   
> +config DRM_PANEL_ANBERNIC_TD4310
> +	tristate "Anbernic TD4310 LCD panel"
> +	depends on GPIOLIB && OF
> +	depends on DRM_MIPI_DSI
> +	depends on BACKLIGHT_CLASS_DEVICE
> +	help
> +	  Say Y here to enable support for Anbernic designed panels with the
> +	  TD4310 panel controller such as the ones used on the Anbernic RG
> +	  Vita Pro.
> +
>   config DRM_PANEL_ARM_VERSATILE
>   	tristate "ARM Versatile panel driver"
>   	depends on OF
> diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
> index a4291dc3905b..9d8f70c9de3e 100644
> --- a/drivers/gpu/drm/panel/Makefile
> +++ b/drivers/gpu/drm/panel/Makefile
> @@ -1,5 +1,6 @@
>   # SPDX-License-Identifier: GPL-2.0
>   obj-$(CONFIG_DRM_PANEL_ABT_Y030XX067A) += panel-abt-y030xx067a.o
> +obj-$(CONFIG_DRM_PANEL_ANBERNIC_TD4310) += panel-anbernic-td4310.o
>   obj-$(CONFIG_DRM_PANEL_ARM_VERSATILE) += panel-arm-versatile.o
>   obj-$(CONFIG_DRM_PANEL_ASUS_Z00T_TM5P5_NT35596) += panel-asus-z00t-tm5p5-n35596.o
>   obj-$(CONFIG_DRM_PANEL_AUO_A030JTN01) += panel-auo-a030jtn01.o
> diff --git a/drivers/gpu/drm/panel/panel-anbernic-td4310.c b/drivers/gpu/drm/panel/panel-anbernic-td4310.c
> new file mode 100644
> index 000000000000..84963671e3f9
> --- /dev/null
> +++ b/drivers/gpu/drm/panel/panel-anbernic-td4310.c
> @@ -0,0 +1,256 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Driver for Anbernic panels with TD4310 panel controller.
> + *
> + * Copyright (C) 2026 Chris Morgan <macromorgan@hotmail.com>
> + *
> + */
> +
> +#include <linux/gpio/consumer.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/regulator/consumer.h>
> +
> +#include <drm/drm_mipi_dsi.h>
> +#include <drm/drm_panel.h>
> +#include <drm/drm_probe_helper.h>
> +
> +#include <video/mipi_display.h>
> +
> +struct anbernic_panel_td4310_info {
> +	const struct drm_display_mode *display_mode;
> +	u16 width_mm;
> +	u16 height_mm;
> +	u32 bus_flags;
> +	unsigned long mode_flags;
> +	u32 format;
> +	u32 lanes;
> +	u16 prepare_delay;
> +	u16 reset_delay;
> +	u16 init_delay;
> +	u16 enable_delay;
> +	u16 disable_delay;
> +	u16 unprepare_delay;
> +};
> +
> +struct anbernic_panel_td4310 {
> +	struct device *dev;
> +	struct mipi_dsi_device *dsi;
> +	struct drm_panel panel;
> +	const struct anbernic_panel_td4310_info *panel_info;
> +	struct gpio_desc *reset_gpio;
> +	struct gpio_desc *enable_gpio;
> +	struct regulator *vdd;
> +	enum drm_panel_orientation orientation;
> +};
> +
> +static inline struct anbernic_panel_td4310 *panel_to_anbernic_panel_td4310(struct drm_panel *panel)
> +{
> +	return container_of(panel, struct anbernic_panel_td4310, panel);
> +}
> +
> +static int panel_anbernic_td4310_prepare(struct drm_panel *panel)
> +{
> +	struct anbernic_panel_td4310 *ctx = panel_to_anbernic_panel_td4310(panel);
> +	struct mipi_dsi_device *dsi = ctx->dsi;
> +	struct mipi_dsi_multi_context dsi_ctx = { .dsi = dsi };
> +	int ret;
> +
> +	ret = regulator_enable(ctx->vdd);
> +	if (ret)
> +		return ret;
> +
> +	ret = gpiod_set_value_cansleep(ctx->enable_gpio, 1);
> +	if (ret)
> +		goto err_enable;
> +
> +	gpiod_set_value_cansleep(ctx->reset_gpio, 0);
> +	mipi_dsi_msleep(&dsi_ctx, 10);
> +
> +	ret = gpiod_set_value_cansleep(ctx->reset_gpio, 1);
> +	if (ret)
> +		goto err_reset;
> +	if (ctx->panel_info->reset_delay)
> +		mipi_dsi_msleep(&dsi_ctx, ctx->panel_info->reset_delay);
> +	ret = gpiod_set_value_cansleep(ctx->reset_gpio, 0);
> +	if (ret)
> +		goto err_reset;
> +
> +	if (ctx->panel_info->enable_delay)
> +		mipi_dsi_msleep(&dsi_ctx, ctx->panel_info->enable_delay);
> +
> +	mipi_dsi_dcs_exit_sleep_mode_multi(&dsi_ctx);
> +	mipi_dsi_msleep(&dsi_ctx, ctx->panel_info->prepare_delay);
> +	mipi_dsi_dcs_set_display_on_multi(&dsi_ctx);
> +	mipi_dsi_msleep(&dsi_ctx, ctx->panel_info->prepare_delay);
> +
> +	if (dsi_ctx.accum_err) {
> +		ret = dsi_ctx.accum_err;
> +		goto err_reset;
> +	}
> +
> +	return 0;
> +
> +err_reset:
> +	gpiod_set_value_cansleep(ctx->enable_gpio, 0);
> +err_enable:
> +	regulator_disable(ctx->vdd);
> +	return ret;
> +}
> +
> +static int panel_anbernic_td4310_unprepare(struct drm_panel *panel)
> +{
> +	struct anbernic_panel_td4310 *ctx = panel_to_anbernic_panel_td4310(panel);
> +	struct mipi_dsi_device *dsi = ctx->dsi;
> +	struct mipi_dsi_multi_context dsi_ctx = { .dsi = dsi };
> +
> +	mipi_dsi_dcs_set_display_off_multi(&dsi_ctx);
> +	mipi_dsi_msleep(&dsi_ctx, ctx->panel_info->unprepare_delay);
> +	mipi_dsi_dcs_enter_sleep_mode_multi(&dsi_ctx);
> +	mipi_dsi_msleep(&dsi_ctx, ctx->panel_info->disable_delay);
> +
> +	gpiod_set_value_cansleep(ctx->enable_gpio, 0);
> +
> +	gpiod_set_value_cansleep(ctx->reset_gpio, 1);
> +
> +	regulator_disable(ctx->vdd);
> +
> +	return 0;
> +}
> +
> +static int panel_anbernic_td4310_get_mode(struct drm_panel *panel,
> +				    struct drm_connector *connector)
> +{
> +	struct anbernic_panel_td4310 *ctx = panel_to_anbernic_panel_td4310(panel);
> +	const struct anbernic_panel_td4310_info *panel_info = ctx->panel_info;
> +
> +	connector->display_info.bpc = 8;
> +	connector->display_info.width_mm = panel_info->width_mm;
> +	connector->display_info.height_mm = panel_info->height_mm;
> +	connector->display_info.bus_flags = panel_info->bus_flags;
> +
> +	return drm_connector_helper_get_modes_fixed(connector, panel_info->display_mode);
> +}
> +
> +static enum drm_panel_orientation panel_anbernic_td4310_get_orientation(struct drm_panel *panel)
> +{
> +	struct anbernic_panel_td4310 *ctx = panel_to_anbernic_panel_td4310(panel);
> +
> +	return ctx->orientation;
> +}
> +
> +static const struct drm_panel_funcs panel_anbernic_td4310_funcs = {
> +	.prepare = panel_anbernic_td4310_prepare,
> +	.unprepare = panel_anbernic_td4310_unprepare,
> +	.get_modes = panel_anbernic_td4310_get_mode,
> +	.get_orientation = panel_anbernic_td4310_get_orientation,
> +};
> +
> +static int panel_anbernic_td4310_probe(struct mipi_dsi_device *dsi)
> +{
> +	struct device *dev = &dsi->dev;
> +	struct anbernic_panel_td4310 *ctx;
> +	int ret;
> +
> +	ctx = devm_drm_panel_alloc(dev, struct anbernic_panel_td4310, panel,
> +				   &panel_anbernic_td4310_funcs,
> +				   DRM_MODE_CONNECTOR_DSI);
> +	if (IS_ERR(ctx))
> +		return PTR_ERR(ctx);
> +
> +	ctx->dev = dev;
> +
> +	ctx->panel_info = of_device_get_match_data(dev);
> +	if (!ctx->panel_info)
> +		return -EINVAL;
> +
> +	ret = of_drm_get_panel_orientation(dev->of_node, &ctx->orientation);
> +	if (ret < 0)
> +		return dev_err_probe(dev, ret, "Failed to get panel orientation\n");
> +
> +	ctx->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
> +	if (IS_ERR(ctx->reset_gpio))
> +		return dev_err_probe(dev, PTR_ERR(ctx->reset_gpio),
> +				     "Cannot get reset gpio\n");
> +
> +	ctx->enable_gpio = devm_gpiod_get_optional(dev, "enable", GPIOD_OUT_LOW);
> +	if (IS_ERR(ctx->enable_gpio))
> +		return dev_err_probe(dev, PTR_ERR(ctx->enable_gpio),
> +				     "Cannot get enable gpio\n");
> +
> +	ctx->vdd = devm_regulator_get(dev, "vdd");
> +	if (IS_ERR(ctx->vdd))
> +		return dev_err_probe(dev, PTR_ERR(ctx->vdd),
> +				     "Failed to request vdd regulator\n");
> +
> +	ctx->dsi = dsi;
> +	mipi_dsi_set_drvdata(dsi, ctx);
> +
> +	dsi->lanes = ctx->panel_info->lanes;
> +	dsi->format = ctx->panel_info->format;
> +	dsi->mode_flags = ctx->panel_info->mode_flags;
> +
> +	ret = drm_panel_of_backlight(&ctx->panel);
> +	if (ret)
> +		return ret;
> +
> +	devm_drm_panel_add(dev, &ctx->panel);
> +
> +	ret = devm_mipi_dsi_attach(dev, dsi);
> +	if (ret < 0)
> +		return dev_err_probe(dev, ret, "Failed to attach to DSI host\n");
> +
> +	return 0;
> +}
> +
> +static const struct drm_display_mode anbernic_vitapro_mode = {
> +	.clock = 140020,
> +	.hdisplay = 1080,
> +	.hsync_start = 1080 + 50,
> +	.hsync_end = 1080 + 50 + 4,
> +	.htotal = 1080 + 50 + 4 + 50,
> +	.vdisplay = 1920,
> +	.vsync_start = 1920 + 15,
> +	.vsync_end = 1920 + 15 + 4,
> +	.vtotal = 1920 + 15 + 4 + 32,
> +	.flags = DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC,
> +};
> +
> +static const struct anbernic_panel_td4310_info anbernic_vitapro_info = {
> +	.display_mode = &anbernic_vitapro_mode,
> +	.width_mm = 69,
> +	.height_mm = 121,
> +	.bus_flags = DRM_BUS_FLAG_DE_LOW | DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE,
> +	.mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_BURST |
> +		      MIPI_DSI_MODE_LPM | MIPI_DSI_MODE_NO_EOT_PACKET |
> +		      MIPI_DSI_CLOCK_NON_CONTINUOUS,
> +	.format = MIPI_DSI_FMT_RGB888,
> +	.lanes = 4,
> +	.prepare_delay = 50,
> +	.reset_delay = 220,
> +	.enable_delay = 120,
> +	.disable_delay = 50,
> +	.unprepare_delay = 20,
> +};
> +
> +static const struct of_device_id panel_anbernic_td4310_of_match[] = {
> +	{
> +		.compatible = "anbernic,panel-vita-pro",
> +		.data = &anbernic_vitapro_info,
> +	},
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, panel_anbernic_td4310_of_match);
> +
> +static struct mipi_dsi_driver anbernic_panel_td4310_driver = {
> +	.driver = {
> +		.name = "panel-anbernic-td4310",
> +		.of_match_table = panel_anbernic_td4310_of_match,
> +	},
> +	.probe	= panel_anbernic_td4310_probe,
> +};
> +module_mipi_dsi_driver(anbernic_panel_td4310_driver);
> +
> +MODULE_AUTHOR("Chris Morgan <macromorgan@hotmail.com>");
> +MODULE_DESCRIPTION("DRM driver for Anbernic TD4310 MIPI DSI panels");
> +MODULE_LICENSE("GPL");

Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>

Thanks,
Neil

^ permalink raw reply

* Re: [PATCH 07/12] rtc: rzn1: fix alarm range check truncation on 32-bit systems
From: Alexandre Belloni @ 2026-06-18 12:26 UTC (permalink / raw)
  To: Lad, Prabhakar
  Cc: Miquel Raynal, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Geert Uytterhoeven, Magnus Damm, Wolfram Sang, linux-rtc,
	linux-renesas-soc, devicetree, linux-kernel, Biju Das,
	Fabrizio Castro, Lad Prabhakar
In-Reply-To: <CA+V-a8uHVR0i+3PY_qi3i0H6fMJSUFb=1cwrgw0VeztVQk-dWw@mail.gmail.com>

On 18/06/2026 11:49:12+0100, Lad, Prabhakar wrote:
> Hi Alexandre,
> 
> On Wed, Jun 17, 2026 at 5:55 PM Alexandre Belloni
> <alexandre.belloni@bootlin.com> wrote:
> >
> > On 15/06/2026 16:48:00+0100, Prabhakar wrote:
> > > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > >
> > > alarm and farest were declared as unsigned long, but
> > > rtc_tm_to_time64() returns time64_t (s64). On 32-bit systems where
> > > unsigned long is 32 bits, the assignment silently truncates the upper
> > > 32 bits of the timestamp.
> > >
> > > Fix by declaring alarm and farest as time64_t and replacing
> > > time_after() with a direct signed comparison, which is correct for
> > > time64_t values that will never realistically overflow.
> > >
> >
> > I'd argue that this is never going to overflow ever as unsigned long
> > gets you to 2106 which is way past the usable range of the RTC so there
> > is a trade off between the size you are going to take on the stack and
> > the actual usefulness of the fix.
> >
> While it's true that unsigned long lasts until 2106 (well past this
> RTC's practical lifetime), rtc_tm_to_time64() explicitly returns
> time64_t. Using unsigned long causes silent truncation and types
> mismatch with the API, which modern static analyzers flag. Given that
> this function is not deeply nested, the 8-byte stack trade-off seems
> worth it for type cleanliness and consistency. What do you think?
> 

I'll take the patch but I still find t a bit irritating that the
justification for this kind of patches is static analyzers report an
issue so let's make the kernel less efficient for everyone.

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply

* Re: [PATCH v8 2/7] mfd: Add driver for ASUS Transformer embedded controller
From: Lee Jones @ 2026-06-18 12:26 UTC (permalink / raw)
  To: Svyatoslav Ryhel
  Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Dmitry Torokhov,
	Pavel Machek, Sebastian Reichel, Ion Agorria,
	Michał Mirosław, devicetree, linux-kernel, linux-input,
	linux-leds, linux-pm
In-Reply-To: <CAPVz0n0caBBt6A+AFeUpGdxvb3Qhoui7khLCt3747bPUKmMXhQ@mail.gmail.com>

On Thu, 11 Jun 2026, Svyatoslav Ryhel wrote:

> чт, 11 черв. 2026 р. о 14:17 Lee Jones <lee@kernel.org> пише:
> >
> > On Thu, 28 May 2026, Svyatoslav Ryhel wrote:
> > > From: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> > >
> > > Support Nuvoton NPCE795-based ECs as used in Asus Transformer TF201,
> > > TF300T, TF300TG, TF300TL and TF700T pad and dock, as well as TF101 dock
> > > and TF600T, P1801-T and TF701T pad. This is a glue driver handling
> > > detection and common operations for EC's functions.
> > >
> > > Co-developed-by: Svyatoslav Ryhel <clamor95@gmail.com>
> > > Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> > > Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> > > ---
> > >  drivers/mfd/Kconfig                     |  16 +
> > >  drivers/mfd/Makefile                    |   1 +
> > >  drivers/mfd/asus-transformer-ec.c       | 542 ++++++++++++++++++++++++
> > >  include/linux/mfd/asus-transformer-ec.h |  92 ++++
> > >  4 files changed, 651 insertions(+)
> > >  create mode 100644 drivers/mfd/asus-transformer-ec.c
> > >  create mode 100644 include/linux/mfd/asus-transformer-ec.h

[...]

> > > +static void asus_ec_clear_buffer(struct asus_ec_data *ddata)
> > > +{
> > > +     int ret, retry = ASUSEC_RSP_BUFFER_SIZE;
> > > +
> > > +     /*
> > > +      * Read the buffer till we get valid data by checking ASUSEC_OBF_MASK
> > > +      * of the status byte or till we reach end of the 256 byte buffer.
> > > +      */
> > > +     while (retry--) {
> > > +             ret = i2c_smbus_read_i2c_block_data(ddata->client, ASUSEC_READ_BUF,
> > > +                                                 ASUSEC_ENTRY_SIZE,
> > > +                                                 ddata->ec_buf);
> > > +             if (ret < ASUSEC_ENTRY_SIZE)
> > > +                     continue;
> > > +
> > > +             if (ddata->ec_buf[ASUSEC_IRQ_STATUS] & ASUSEC_OBF_MASK)
> > > +                     continue;
> > > +
> > > +             break;
> > > +     }
> > > +}
> > > +
> > > +static int asus_ec_log_info(struct asus_ec_data *ddata, unsigned int reg,
> > > +                         const char *name, const char **out)

If we can avoid points to pointers, then please do.

We already have ddata, so we can just set the name?

It will remove a lot of the following complexity / ugliness.

> > > +{
> > > +     struct device *dev = &ddata->client->dev;
> > > +     u8 buf[ASUSEC_ENTRY_BUFSIZE];
> > > +     int ret;
> > > +
> > > +     memset(buf, 0, ASUSEC_ENTRY_BUFSIZE);
> > > +     ret = i2c_smbus_read_i2c_block_data(ddata->ec.dockram, reg,
> > > +                                         ASUSEC_ENTRY_SIZE, buf);
> > > +     if (ret < ASUSEC_ENTRY_SIZE)
> > > +             return ret;
> >
> > Same here.  These should be negative.
> >
> 
> return ret < 0 ? ret : -EIO same as above
> 
> > > +
> > > +     if (buf[0] > ASUSEC_ENTRY_SIZE) {
> > > +             dev_err(dev, "bad data len; buffer: %*ph; ret: %d\n",
> > > +                     ASUSEC_ENTRY_BUFSIZE, buf, ret);
> > > +             return -EPROTO;
> > > +     }
> > > +
> > > +     if (!ddata->logging_disabled) {
> > > +             dev_info(dev, "%-14s: %.*s\n", name, buf[0], buf + 1);
> > > +
> > > +             if (out) {
> > > +                     *out = devm_kasprintf(dev, GFP_KERNEL, "%.*s",
> > > +                                           buf[0], buf + 1);
> > > +                     if (!*out)
> > > +                             return -ENOMEM;
> > > +             }
> > > +     }
> >
> > FWIW, I hate this!  What does it give you now that development is done?
> >
> 
> We have already discussed this, and you agreed that EC and firmware
> prints may stay! This prints EC model and firmware info as well as EC
> firmware behavior. It allows identify possible new revisions of EC -
> Firmware combo and address possible regressions (check if it is chip
> malfunction or firmware needs a new programming model) without
> rebuilding kernel and digging downstream kernel for needed bits of
> code.

Right, so just print it out and remove all of the 'logging_disabled' and
'out' nonsense.

> > > +     return 0;
> > > +}
> > > +
> > > +static int asus_ec_reset(struct asus_ec_data *ddata)
> > > +{
> > > +     int retry, ret;
> > > +
> > > +     guard(mutex)(&ddata->ecreq_lock);
> > > +
> > > +     for (retry = 0; retry < ASUSEC_RETRY_MAX; retry++) {
> >
> > for (int retry = ... is generally preferred for throwaway variables.
> >
> 
> Not that I care too much, but I am defining ret anyway, why not add
> retry too there?

This is the new and preferred way to use throw-away variables.

ret is not a throw-away variable.

[...]

> > > +static int asus_ec_set_factory_mode(struct asus_ec_data *ddata,
> > > +                                 enum asusec_mode fmode)
> > > +{
> > > +     dev_info(&ddata->client->dev, "Entering %s mode.\n",
> > > +              fmode == ASUSEC_MODE_FACTORY ? "factory" : "normal");
> > > +
> > > +     return asus_dockram_access_ctl(ddata->ec.dockram, NULL,
> > > +                                    ASUSEC_CTL_FACTORY_MODE,
> > > +                                    fmode == ASUSEC_MODE_FACTORY ?
> > > +                                    ASUSEC_CTL_FACTORY_MODE : 0);
> >
> > Why not create make:
> >
> > ASUSEC_MODE_FACTORY == ASUSEC_CTL_FACTORY_MODE
> >
> > What happens to NORMAL?
> >
> 
> ASUSEC_CTL_FACTORY_MODE is a bit in the ctl register. For NORMAL mode

I get that, but if the values can be shared, it make the code simpler.

> bit is cleared,
> for FACTORY bit it set, for NONE bit is ignored.
> 
> > > +}
> > > +
> > > +static int asus_ec_detect(struct asus_ec_data *ddata)
> > > +{
> > > +     int ret;
> > > +
> > > +     ret = asus_ec_reset(ddata);
> > > +     if (ret)
> > > +             goto err_exit;
> > > +
> > > +     asus_ec_clear_buffer(ddata);
> > > +
> > > +     ret = asus_ec_log_info(ddata, ASUSEC_DOCKRAM_INFO_MODEL, "Model",
> > > +                            &ddata->ec.model);

Where is this model used?

> > You can use 100-chars and make the code look beautiful! :)
> >
> 
> Not every subsystem permits 100 chars, some stick to 80 as a strict
> rule, so it is better be safe.

Right, but we are forward thinking here.

You can and should use 100-chars in this subsystem.

> > > +     if (ret)
> > > +             goto err_exit;
> > > +
> > > +     ret = asus_ec_log_info(ddata, ASUSEC_DOCKRAM_INFO_FW, "FW version",
> > > +                            NULL);
> > > +     if (ret)
> > > +             goto err_exit;
> > > +
> > > +     ret = asus_ec_log_info(ddata, ASUSEC_DOCKRAM_INFO_CFGFMT, "Config format",
> > > +                            NULL);
> > > +     if (ret)
> > > +             goto err_exit;
> > > +
> > > +     ret = asus_ec_log_info(ddata, ASUSEC_DOCKRAM_INFO_HW, "HW version",
> > > +                            NULL);
> > > +     if (ret)
> > > +             goto err_exit;
> > > +
> > > +     /* Disable logging on next EC request */
> >
> > Why, but why?
> >
> 
> Cause EC requests are frequent (handshake/reset) and constant logging
> same data is not acceptable.

Then rid the prints entirely or do them at a more appropriate point like
during probe?

Or maybe consider dev_info_once() and friends.

> > > +     ddata->logging_disabled = true;
> > > +
> > > +     /* Check and inform about EC firmware behavior */
> > > +     ret = asus_ec_susb_on_status(ddata);
> > > +     if (ret)
> > > +             goto err_exit;
> > > +
> > > +     ddata->ec.name = ddata->info->name;
> > > +
> > > +     /* Some EC require factory mode to be set normal on each request */
> > > +     if (ddata->info->fmode)
> > > +             ret = asus_ec_set_factory_mode(ddata, ddata->info->fmode);
> > > +
> > > +err_exit:
> > > +     if (ret)
> > > +             dev_err(&ddata->client->dev, "failed to access EC: %d\n", ret);
> > > +
> > > +     return ret;
> > > +}
> > > +
> > > +static void asus_ec_handle_smi(struct asus_ec_data *ddata, unsigned int code)
> > > +{
> > > +     switch (code) {
> > > +     case ASUSEC_SMI_HANDSHAKE:
> > > +     case ASUSEC_SMI_RESET:
> > > +             asus_ec_detect(ddata);
> > > +             break;
> > > +     }
> > > +}
> > > +
> > > +static irqreturn_t asus_ec_interrupt(int irq, void *dev_id)
> > > +{
> > > +     struct asus_ec_data *ddata = dev_id;
> > > +     unsigned long notify_action;
> > > +     int ret;
> > > +
> > > +     ret = i2c_smbus_read_i2c_block_data(ddata->client, ASUSEC_READ_BUF,
> > > +                                         ASUSEC_ENTRY_SIZE, ddata->ec_buf);
> > > +     if (ret < ASUSEC_ENTRY_SIZE ||
> > > +         !(ddata->ec_buf[ASUSEC_IRQ_STATUS] & ASUSEC_OBF_MASK))
> >
> > Unwrap for readability.
> >
> > Also, I think a comment would be helpful.
> >
> 
> if (ret < ASUSEC_ENTRY_SIZE)
>     return IRQ_NONE;
> 
> ret = ddata->ec_buf[ASUSEC_IRQ_STATUS] & ASUSEC_OBF_MASK;
> if (!ret)
>     return IRQ_NONE;
> 
> This would be acceptable? (I will add comments later on)

Yes, better.

If you're not using ret again, you could just put 'ddata.." inside the if().

> > > +             return IRQ_NONE;
> > > +
> > > +     notify_action = ddata->ec_buf[ASUSEC_IRQ_STATUS];
> > > +     if (notify_action & ASUSEC_SMI_MASK) {
> > > +             unsigned int code = ddata->ec_buf[ASUSEC_SMI_CODE];
> > > +
> > > +             asus_ec_handle_smi(ddata, code);
> > > +
> > > +             notify_action |= code << 8;
> > > +     }
> > > +
> > > +     blocking_notifier_call_chain(&ddata->ec.notify_list,
> > > +                                  notify_action, ddata->ec_buf);
> > > +
> > > +     return IRQ_HANDLED;
> > > +}
> > > +
> > > +static void asus_ec_release_dockram_dev(void *client)
> > > +{
> > > +     i2c_unregister_device(client);
> > > +}
> > > +
> > > +static struct i2c_client *devm_asus_dockram_get(struct device *dev)
> > > +{
> > > +     struct i2c_client *parent = to_i2c_client(dev);
> > > +     struct i2c_client *dockram;
> > > +     struct dockram_ec_data *ddata;
> > > +     int ret;
> > > +
> > > +     dockram = i2c_new_ancillary_device(parent, "dockram",
> > > +                                        parent->addr + 2);
> >
> > Could we define a macro for the address offset '2' here to avoid using a magic
> > number?
> >
> 
> It seems that you are excessively concerned with "magic numbers".

Bingo!  I HATE magic numbers.

https://lore.kernel.org/all/?q=%22Lee+Jones%22+magic

~900 messages!  =:-D

[...]

> > > +static const struct asus_ec_chip_info asus_ec_tf600t_pad_data = {
> > > +     .name = "pad",
> > > +     .variant = ASUSEC_TF600T_PAD,
> > > +     .fmode = ASUSEC_MODE_NORMAL,
> > > +};
> >
> > Any reason not to just pass the identifier (variant) and add the name
> > and fmode attribues to the switch() above?
> 
> Why not set it here, I am not passing any mfd or any other API via of data.

I get that, and you're not breaking any of my golden rules.

However, I just think doing everything in one place, usually a which
based off of the 'variant' which you pass as a single value, is a nicer,
more consolidated way of doing things.

-- 
Lee Jones

^ permalink raw reply

* Re: [PATCH 3/3] drm/panel: himax-hx83121a: add backlight regulator support
From: Neil Armstrong @ 2026-06-18 12:23 UTC (permalink / raw)
  To: Icenowy Zheng, Jessica Zhang, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Pengyu Luo
  Cc: Liam Girdwood, Mark Brown, dri-devel, devicetree, linux-kernel
In-Reply-To: <20260608162622.403713-3-zhengxingda@iscas.ac.cn>

On 6/8/26 18:26, Icenowy Zheng wrote:
> The backlight, when managed by the panel controller, could be powered by
> an external regulator, and shutting down the regulator could power off
> the backlight.
> 
> Add support for such a regulator. It's powered off when the backlight is
> 0 (either by setting brightness to 0 or setting bl_power), and powered
> on when the backlight should be operating.
> 
> Signed-off-by: Icenowy Zheng <zhengxingda@iscas.ac.cn>
> ---
>   drivers/gpu/drm/panel/panel-himax-hx83121a.c | 26 ++++++++++++++++++++
>   1 file changed, 26 insertions(+)
> 
> diff --git a/drivers/gpu/drm/panel/panel-himax-hx83121a.c b/drivers/gpu/drm/panel/panel-himax-hx83121a.c
> index 1a7e0125bced8..e31e2fba0a787 100644
> --- a/drivers/gpu/drm/panel/panel-himax-hx83121a.c
> +++ b/drivers/gpu/drm/panel/panel-himax-hx83121a.c
> @@ -34,7 +34,9 @@ struct himax {
>   	struct drm_dsc_config dsc;
>   	struct gpio_desc *reset_gpio;
>   	struct regulator_bulk_data *supplies;
> +	struct regulator *bl_supply;
>   	struct backlight_device *backlight;
> +	bool backlight_enabled;
>   };
>   
>   struct panel_desc {
> @@ -195,7 +197,27 @@ static int himax_bl_update_status(struct backlight_device *bl)
>   {
>   	struct himax *ctx = bl_get_data(bl);
>   	u16 brightness = backlight_get_brightness(bl);
> +	int ret = 0;
> +
> +	if (!brightness) {
> +		if (ctx->backlight_enabled)
> +			ret = regulator_disable(ctx->bl_supply);
> +		if (ret)
> +			return ret;
> +
> +		ctx->backlight_enabled = false;
> +
> +		return 0;
> +	}
> +
>   	/* TODO: brightness to raw map table */
> +	if (!ctx->backlight_enabled)
> +		ret = regulator_enable(ctx->bl_supply);
> +	if (ret)
> +		return ret;
> +
> +	ctx->backlight_enabled = true;
> +
>   	return mipi_dsi_dcs_set_display_brightness_large(to_primary_dsi(ctx),
>   							 brightness);
>   }
> @@ -647,6 +669,10 @@ static int himax_probe(struct mipi_dsi_device *dsi)
>   	ctx->panel.prepare_prev_first = true;
>   
>   	if (desc->has_dcs_backlight) {
> +		ctx->bl_supply = devm_regulator_get_optional(dev, "bl");
> +		if (IS_ERR(ctx->bl_supply))
> +			return dev_err_probe(dev, PTR_ERR(ctx->bl_supply),
> +					     "Failed to get backlight supply\n");
>   		ctx->backlight = himax_create_backlight(ctx);
>   		if (IS_ERR(ctx->backlight))
>   			return dev_err_probe(dev, PTR_ERR(ctx->backlight),

Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>

Thanks,
Neil

^ permalink raw reply

* Re: [PATCH 2/3] drm/panel: himax-hx83121a: pass the panel pointer when creating BL
From: Neil Armstrong @ 2026-06-18 12:23 UTC (permalink / raw)
  To: Icenowy Zheng, Jessica Zhang, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Pengyu Luo
  Cc: Liam Girdwood, Mark Brown, dri-devel, devicetree, linux-kernel
In-Reply-To: <20260608162622.403713-2-zhengxingda@iscas.ac.cn>

On 6/8/26 18:26, Icenowy Zheng wrote:
> As backlight powering on/off support will be added, more fields of the
> panel context will be accessed in the backlight update function.
> 
> Pass the whole panel struct instead of the DSI device when creating the
> backlight device.
> 
> Signed-off-by: Icenowy Zheng <zhengxingda@iscas.ac.cn>
> ---
>   drivers/gpu/drm/panel/panel-himax-hx83121a.c | 13 +++++++------
>   1 file changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/panel/panel-himax-hx83121a.c b/drivers/gpu/drm/panel/panel-himax-hx83121a.c
> index bed79aa06f46a..1a7e0125bced8 100644
> --- a/drivers/gpu/drm/panel/panel-himax-hx83121a.c
> +++ b/drivers/gpu/drm/panel/panel-himax-hx83121a.c
> @@ -193,10 +193,11 @@ static const struct drm_panel_funcs himax_panel_funcs = {
>   
>   static int himax_bl_update_status(struct backlight_device *bl)
>   {
> -	struct mipi_dsi_device *dsi = bl_get_data(bl);
> +	struct himax *ctx = bl_get_data(bl);
>   	u16 brightness = backlight_get_brightness(bl);
>   	/* TODO: brightness to raw map table */
> -	return mipi_dsi_dcs_set_display_brightness_large(dsi, brightness);
> +	return mipi_dsi_dcs_set_display_brightness_large(to_primary_dsi(ctx),
> +							 brightness);
>   }
>   
>   static const struct backlight_ops himax_bl_ops = {
> @@ -205,9 +206,9 @@ static const struct backlight_ops himax_bl_ops = {
>   };
>   
>   static struct backlight_device *
> -himax_create_backlight(struct mipi_dsi_device *dsi)
> +himax_create_backlight(struct himax *ctx)
>   {
> -	struct device *dev = &dsi->dev;
> +	struct device *dev = &to_primary_dsi(ctx)->dev;
>   	const struct backlight_properties props = {
>   		.type = BACKLIGHT_RAW,
>   		.brightness = 512,
> @@ -215,7 +216,7 @@ himax_create_backlight(struct mipi_dsi_device *dsi)
>   		.scale = BACKLIGHT_SCALE_NON_LINEAR,
>   	};
>   
> -	return devm_backlight_device_register(dev, dev_name(dev), dev, dsi,
> +	return devm_backlight_device_register(dev, dev_name(dev), dev, ctx,
>   					      &himax_bl_ops, &props);
>   }
>   
> @@ -646,7 +647,7 @@ static int himax_probe(struct mipi_dsi_device *dsi)
>   	ctx->panel.prepare_prev_first = true;
>   
>   	if (desc->has_dcs_backlight) {
> -		ctx->backlight = himax_create_backlight(to_primary_dsi(ctx));
> +		ctx->backlight = himax_create_backlight(ctx);
>   		if (IS_ERR(ctx->backlight))
>   			return dev_err_probe(dev, PTR_ERR(ctx->backlight),
>   					     "Failed to create backlight\n");

Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>

Thanks,
Neil

^ permalink raw reply

* Re: [PATCH v2 2/2] arm64: dts: qcom: glymur: add TRNG node
From: Konrad Dybcio @ 2026-06-18 12:20 UTC (permalink / raw)
  To: Harshal Dev, Bjorn Andersson
  Cc: Kuldeep Singh, linux-arm-msm, linux-crypto, devicetree,
	linux-kernel, Herbert Xu, David S. Miller, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Vinod Koul, Konrad Dybcio,
	Dmitry Baryshkov
In-Reply-To: <b4794e93-0fd3-4559-9ecd-02d679bd06b5@oss.qualcomm.com>

On 6/18/26 1:58 PM, Harshal Dev wrote:
> Hi Bjorn,
> 
> On 6/9/2026 12:06 PM, Harshal Dev wrote:
>> Hello Bjorn,
>>
>> On 5/18/2026 2:06 PM, Harshal Dev wrote:
>>> Hi Bjorn,
>>>
>>> On 4/24/2026 2:05 PM, Harshal Dev wrote:
>>>> Glymur has a True Random Number Generator, add the node with the correct
>>>> compatible set.
>>>>
>>>> Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
>>>> Signed-off-by: Harshal Dev <harshal.dev@oss.qualcomm.com>
>>>> ---
>>>>  arch/arm64/boot/dts/qcom/glymur.dtsi | 5 +++++
>>>>  1 file changed, 5 insertions(+)
>>>>
>>>> diff --git a/arch/arm64/boot/dts/qcom/glymur.dtsi b/arch/arm64/boot/dts/qcom/glymur.dtsi
>>>> index f23cf81ddb77..64bbd5691229 100644
>>>> --- a/arch/arm64/boot/dts/qcom/glymur.dtsi
>>>> +++ b/arch/arm64/boot/dts/qcom/glymur.dtsi
>>>> @@ -3675,6 +3675,11 @@ pcie3b_phy: phy@f10000 {
>>>>  			status = "disabled";
>>>>  		};
>>>>  
>>>> +		rng: rng@10c3000 {
>>>> +			compatible = "qcom,glymur-trng", "qcom,trng";
>>>> +			reg = <0x0 0x010c3000 0x0 0x1000>;
>>>> +		};
>>>> +
>>>>  		tcsr_mutex: hwlock@1f40000 {
>>>>  			compatible = "qcom,tcsr-mutex";
>>>>  			reg = <0x0 0x01f40000 0x0 0x20000>;
>>>>
>>>
>>> A gentle reminder to pick this patch for the 7.2 merge window.
>>>
>>
>> Another reminder to pick this patch up in-case you've missed it.
>>
> 
> Gentle reminder.

Bjorn and I were both out at the time, after we returned it was too
late to accept new patches.

Currently we're halfway through the merge window (where Torvalds
receives pull requests to create 7.2-rc1 out of), during which
contributions are not accepted. They will resume in ~1.5wk after
7.2-rc1 is tagged, targetting 7.3

Konrad

^ permalink raw reply

* Re: [PATCH v6 2/2] drm/panel: Add Ilitek ILI9488 controller driver
From: Neil Armstrong @ 2026-06-18 12:15 UTC (permalink / raw)
  To: Igor Reznichenko, Jessica Zhang, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Heiko Stuebner, Lad Prabhakar,
	Manivannan Sadhasivam, Kael D'Alcamo, Kever Yang
  Cc: dri-devel, devicetree, linux-kernel
In-Reply-To: <20260617143245.372182-3-igor@reznichenko.net>

On 6/17/26 16:32, Igor Reznichenko wrote:
> Add support for Ilitek ILI9488 DSI controller which is used in
> FocusLCDs E35GH-I-MW800-CB 320x480 MIPI DSI panel. The mode timing
> was adjusted after STM32MP157 testing. The previous 14.256 MHz mode
> worked on AM62P, but was awkward for STM32 DSI/LTDC clock synthesis.
> 
> Signed-off-by: Igor Reznichenko <igor@reznichenko.net>
> ---
>   MAINTAINERS                                  |   6 +
>   drivers/gpu/drm/panel/Kconfig                |   9 +
>   drivers/gpu/drm/panel/Makefile               |   1 +
>   drivers/gpu/drm/panel/panel-ilitek-ili9488.c | 289 +++++++++++++++++++
>   4 files changed, 305 insertions(+)
>   create mode 100644 drivers/gpu/drm/panel/panel-ilitek-ili9488.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index ac87c217ab1f..987635948cde 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -8040,6 +8040,12 @@ T:	git https://gitlab.freedesktop.org/drm/misc/kernel.git
>   F:	Documentation/devicetree/bindings/display/ilitek,ili9486.yaml
>   F:	drivers/gpu/drm/tiny/ili9486.c
>   
> +DRM DRIVER FOR ILITEK ILI9488 PANELS
> +M:	Igor Reznichenko <igor@reznichenko.net>
> +S:	Maintained
> +F:	Documentation/devicetree/bindings/display/panel/ilitek,ili9488.yaml
> +F:	drivers/gpu/drm/panel/panel-ilitek-ili9488.c
> +
>   DRM DRIVER FOR ILITEK ILI9805 PANELS
>   M:	Michael Trimarchi <michael@amarulasolutions.com>
>   S:	Maintained
> diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
> index 7450b27622a2..37987e47f3f7 100644
> --- a/drivers/gpu/drm/panel/Kconfig
> +++ b/drivers/gpu/drm/panel/Kconfig
> @@ -284,6 +284,15 @@ config DRM_PANEL_ILITEK_ILI9341
>   	  QVGA (240x320) RGB panels. support serial & parallel rgb
>   	  interface.
>   
> +config DRM_PANEL_ILITEK_ILI9488
> +	tristate "Ilitek ILI9488-based panels"
> +	depends on OF
> +	depends on DRM_MIPI_DSI
> +	depends on BACKLIGHT_CLASS_DEVICE
> +	help
> +	  Say Y if you want to enable support for panels based on the
> +	  Ilitek ILI9488 controller.
> +
>   config DRM_PANEL_ILITEK_ILI9805
>   	tristate "Ilitek ILI9805-based panels"
>   	depends on OF
> diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
> index c2c5cf817116..a1344a0a3fd0 100644
> --- a/drivers/gpu/drm/panel/Makefile
> +++ b/drivers/gpu/drm/panel/Makefile
> @@ -28,6 +28,7 @@ obj-$(CONFIG_DRM_PANEL_HIMAX_HX8394) += panel-himax-hx8394.o
>   obj-$(CONFIG_DRM_PANEL_HYDIS_HV101HD1) += panel-hydis-hv101hd1.o
>   obj-$(CONFIG_DRM_PANEL_ILITEK_IL9322) += panel-ilitek-ili9322.o
>   obj-$(CONFIG_DRM_PANEL_ILITEK_ILI9341) += panel-ilitek-ili9341.o
> +obj-$(CONFIG_DRM_PANEL_ILITEK_ILI9488) += panel-ilitek-ili9488.o
>   obj-$(CONFIG_DRM_PANEL_ILITEK_ILI9805) += panel-ilitek-ili9805.o
>   obj-$(CONFIG_DRM_PANEL_ILITEK_ILI9806E_CORE) += panel-ilitek-ili9806e-core.o
>   obj-$(CONFIG_DRM_PANEL_ILITEK_ILI9806E_DSI) += panel-ilitek-ili9806e-dsi.o
> diff --git a/drivers/gpu/drm/panel/panel-ilitek-ili9488.c b/drivers/gpu/drm/panel/panel-ilitek-ili9488.c
> new file mode 100644
> index 000000000000..7302766034fc
> --- /dev/null
> +++ b/drivers/gpu/drm/panel/panel-ilitek-ili9488.c
> @@ -0,0 +1,289 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include <linux/delay.h>
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/errno.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +
> +#include <linux/gpio/consumer.h>
> +#include <linux/regulator/consumer.h>
> +
> +#include <drm/drm_mipi_dsi.h>
> +#include <drm/drm_modes.h>
> +#include <drm/drm_panel.h>
> +#include <drm/drm_probe_helper.h>
> +
> +#include <video/mipi_display.h>
> +
> +struct ili9488_desc {
> +	const struct drm_display_mode *display_mode;
> +	unsigned long mode_flags;
> +	enum mipi_dsi_pixel_format format;
> +	unsigned int lanes;
> +	unsigned int bpc;
> +	void (*init_sequence)(struct mipi_dsi_multi_context *ctx);
> +};
> +
> +struct ili9488 {
> +	struct drm_panel panel;
> +	struct mipi_dsi_device *dsi;
> +	struct gpio_desc *reset;
> +	struct regulator_bulk_data supplies[2];
> +	const struct ili9488_desc *desc;
> +	enum drm_panel_orientation orientation;
> +};
> +
> +static const char * const regulator_names[] = {
> +	"vci",
> +	"iovcc",
> +};
> +
> +static void e35gh_i_mw800cb_init(struct mipi_dsi_multi_context *ctx)
> +{
> +	/* Gamma control 1,2 */
> +	mipi_dsi_dcs_write_seq_multi(ctx, 0xE0, 0x00, 0x10, 0x14, 0x01, 0x0E, 0x04, 0x33,
> +				     0x56, 0x48, 0x03, 0x0C, 0x0B, 0x2B, 0x34, 0x0F);
> +	mipi_dsi_dcs_write_seq_multi(ctx, 0xE1, 0x00, 0x12, 0x18, 0x05, 0x12, 0x06, 0x40,
> +				     0x34, 0x57, 0x06, 0x10, 0x0C, 0x3B, 0x3F, 0x0F);
> +	/* Power control 1,2 */
> +	mipi_dsi_dcs_write_seq_multi(ctx, 0xC0, 0x0F, 0x0C);
> +	mipi_dsi_dcs_write_seq_multi(ctx, 0xC1, 0x41);
> +	/* VCOM Control */
> +	mipi_dsi_dcs_write_seq_multi(ctx, 0xC5, 0x00, 0x25, 0x80);
> +	mipi_dsi_dcs_write_seq_multi(ctx, 0x36, 0x48);
> +	/* Interface pixel format 18bpp */
> +	mipi_dsi_dcs_write_seq_multi(ctx, 0x3A, 0x66);
> +	mipi_dsi_dcs_write_seq_multi(ctx, 0xB0, 0x00);
> +	mipi_dsi_dcs_write_seq_multi(ctx, 0xB1, 0xA0);
> +	mipi_dsi_dcs_write_seq_multi(ctx, 0xB4, 0x02);
> +	mipi_dsi_dcs_write_seq_multi(ctx, 0xB6, 0x02, 0x02, 0x3B);
> +	mipi_dsi_dcs_write_seq_multi(ctx, 0xE9, 0x00);
> +	mipi_dsi_dcs_write_seq_multi(ctx, 0xF7, 0xA9, 0x51, 0x2C, 0x82);
> +	mipi_dsi_dcs_write_seq_multi(ctx, 0x21);
> +}
> +
> +static const struct drm_display_mode e35gh_i_mw800cb_display_mode = {
> +	.clock = 14400,
> +
> +	.hdisplay = 320,
> +	.hsync_start = 320 + 60,
> +	.hsync_end = 320 + 60 + 20,
> +	.htotal = 320 + 60 + 20 + 42,
> +
> +	.vdisplay = 480,
> +	.vsync_start = 480 + 20,
> +	.vsync_end = 480 + 20 + 10,
> +	.vtotal = 480 + 20 + 10 + 33,
> +
> +	.width_mm = 48,
> +	.height_mm = 73,
> +
> +	.flags = DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC,
> +	.type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED,
> +};
> +
> +static inline struct ili9488 *panel_to_ili9488(struct drm_panel *panel)
> +{
> +	return container_of(panel, struct ili9488, panel);
> +}
> +
> +static int ili9488_power_on(struct ili9488 *ili)
> +{
> +	struct mipi_dsi_device *dsi = ili->dsi;
> +	int ret;
> +
> +	ret = regulator_bulk_enable(ARRAY_SIZE(ili->supplies), ili->supplies);
> +	if (ret < 0) {
> +		dev_err(&dsi->dev, "regulator bulk enable failed: %d\n", ret);
> +		return ret;
> +	}
> +
> +	gpiod_set_value_cansleep(ili->reset, 0);
> +	usleep_range(1000, 5000);
> +	gpiod_set_value_cansleep(ili->reset, 1);
> +	usleep_range(1000, 5000);
> +	gpiod_set_value_cansleep(ili->reset, 0);
> +	usleep_range(5000, 10000);
> +
> +	return 0;
> +}
> +
> +static int ili9488_power_off(struct ili9488 *ili)
> +{
> +	struct mipi_dsi_device *dsi = ili->dsi;
> +	int ret;
> +
> +	gpiod_set_value_cansleep(ili->reset, 1);
> +
> +	ret = regulator_bulk_disable(ARRAY_SIZE(ili->supplies), ili->supplies);
> +	if (ret)
> +		dev_err(&dsi->dev, "regulator bulk disable failed: %d\n", ret);
> +
> +	return ret;
> +}
> +
> +static int ili9488_activate(struct ili9488 *ili)
> +{
> +	struct mipi_dsi_multi_context ctx = { .dsi = ili->dsi };
> +
> +	if (ili->desc->init_sequence)
> +		ili->desc->init_sequence(&ctx);
> +
> +	mipi_dsi_dcs_exit_sleep_mode_multi(&ctx);
> +	mipi_dsi_msleep(&ctx, 120);
> +	mipi_dsi_dcs_set_display_on_multi(&ctx);
> +
> +	return ctx.accum_err;
> +}
> +
> +static int ili9488_prepare(struct drm_panel *panel)
> +{
> +	struct ili9488 *ili = panel_to_ili9488(panel);
> +	int ret;
> +
> +	ret = ili9488_power_on(ili);
> +	if (ret)
> +		return ret;
> +
> +	ret = ili9488_activate(ili);
> +	if (ret) {
> +		ili9488_power_off(ili);
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static int ili9488_deactivate(struct ili9488 *ili)
> +{
> +	struct mipi_dsi_multi_context ctx = { .dsi = ili->dsi };
> +
> +	mipi_dsi_dcs_set_display_off_multi(&ctx);
> +	mipi_dsi_dcs_enter_sleep_mode_multi(&ctx);
> +	mipi_dsi_msleep(&ctx, 120);
> +
> +	return ctx.accum_err;
> +}
> +
> +static int ili9488_unprepare(struct drm_panel *panel)
> +{
> +	struct ili9488 *ili = panel_to_ili9488(panel);
> +	struct mipi_dsi_device *dsi = ili->dsi;
> +	int ret;
> +
> +	ili9488_deactivate(ili);
> +	ret = ili9488_power_off(ili);
> +	if (ret < 0)
> +		dev_err(&dsi->dev, "power off failed: %d\n", ret);
> +
> +	return ret;
> +}
> +
> +static int ili9488_get_modes(struct drm_panel *panel, struct drm_connector *connector)
> +{
> +	struct ili9488 *ili = panel_to_ili9488(panel);
> +	const struct drm_display_mode *mode = ili->desc->display_mode;
> +
> +	connector->display_info.bpc = ili->desc->bpc;
> +
> +	return drm_connector_helper_get_modes_fixed(connector, mode);
> +}
> +
> +static enum drm_panel_orientation ili9488_get_orientation(struct drm_panel *panel)
> +{
> +	struct ili9488 *ili = panel_to_ili9488(panel);
> +
> +	return ili->orientation;
> +}
> +
> +static const struct drm_panel_funcs ili9488_funcs = {
> +	.prepare	= ili9488_prepare,
> +	.unprepare	= ili9488_unprepare,
> +	.get_modes	= ili9488_get_modes,
> +	.get_orientation = ili9488_get_orientation,
> +};
> +
> +static int ili9488_dsi_probe(struct mipi_dsi_device *dsi)
> +{
> +	struct device *dev = &dsi->dev;
> +	struct ili9488 *ili;
> +	int i, ret;
> +
> +	ili = devm_drm_panel_alloc(dev, struct ili9488, panel, &ili9488_funcs,
> +				   DRM_MODE_CONNECTOR_DSI);
> +	if (IS_ERR(ili))
> +		return PTR_ERR(ili);
> +
> +	ili->desc = device_get_match_data(dev);
> +	ili->dsi = dsi;
> +
> +	dsi->mode_flags = ili->desc->mode_flags;
> +	dsi->format = ili->desc->format;
> +	dsi->lanes = ili->desc->lanes;
> +
> +	ili->reset = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
> +	if (IS_ERR(ili->reset))
> +		return dev_err_probe(dev, PTR_ERR(ili->reset),
> +				     "failed to get reset-gpios\n");
> +
> +	for (i = 0; i < ARRAY_SIZE(ili->supplies); i++)
> +		ili->supplies[i].supply = regulator_names[i];
> +
> +	ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(ili->supplies),
> +				      ili->supplies);
> +	if (ret < 0)
> +		return dev_err_probe(dev, ret, "failed to get regulators\n");
> +
> +	ret = of_drm_get_panel_orientation(dev->of_node, &ili->orientation);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "failed to get orientation\n");
> +
> +	ret = drm_panel_of_backlight(&ili->panel);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "failed to get backlight\n");
> +
> +	ili->panel.prepare_prev_first = true;
> +
> +	ret = devm_drm_panel_add(dev, &ili->panel);
> +	if (ret)
> +		return ret;
> +
> +	ret = devm_mipi_dsi_attach(dev, dsi);
> +	if (ret < 0)
> +		return dev_err_probe(dev, ret, "failed to attach to DSI host\n");
> +
> +	return 0;
> +}
> +
> +static const struct ili9488_desc e35gh_i_mw800cb_desc = {
> +	.init_sequence = e35gh_i_mw800cb_init,
> +	.display_mode = &e35gh_i_mw800cb_display_mode,
> +	.mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_SYNC_PULSE |
> +		      MIPI_DSI_MODE_LPM | MIPI_DSI_CLOCK_NON_CONTINUOUS,
> +	.format = MIPI_DSI_FMT_RGB666_PACKED,
> +	.lanes = 1,
> +	.bpc = 6,
> +};
> +
> +static const struct of_device_id ili9488_of_match[] = {
> +	{ .compatible = "focuslcds,e35gh-i-mw800cb", .data = &e35gh_i_mw800cb_desc },
> +	{ }
> +};
> +
> +MODULE_DEVICE_TABLE(of, ili9488_of_match);
> +
> +static struct mipi_dsi_driver ili9488_dsi_driver = {
> +	.probe	= ili9488_dsi_probe,
> +	.driver = {
> +		.name		= "ili9488-dsi",
> +		.of_match_table	= ili9488_of_match,
> +	},
> +};
> +module_mipi_dsi_driver(ili9488_dsi_driver);
> +
> +MODULE_AUTHOR("Igor Reznichenko <igor@reznichenko.net>");
> +MODULE_DESCRIPTION("Ilitek ILI9488 Controller Driver");
> +MODULE_LICENSE("GPL");

Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>

Thanks,
Neil

^ permalink raw reply

* Re: [PATCH v2 1/3] arm64: dts: qcom: sm8750: Add camss node
From: sashiko-bot @ 2026-06-18 12:14 UTC (permalink / raw)
  To: Hangxiang Ma; +Cc: robh, conor+dt, devicetree
In-Reply-To: <20260618-pakala-camera-v2-1-9fbb729fd242@oss.qualcomm.com>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The patch introduces a new device tree node with the compatible string 'qcom,sm8750-camss' but fails to provide the required dt-binding documentation.
--

commit b023f9563ab4fb78586aec9991211a5cce1748d9
Author: Hangxiang Ma <hangxiang.ma@oss.qualcomm.com>

arm64: dts: qcom: sm8750: Add camss node

This patch introduces a device tree node for the SM8750 camera subsystem
(camss).

> diff --git a/arch/arm64/boot/dts/qcom/sm8750.dtsi b/arch/arm64/boot/dts/qcom/sm8750.dtsi
> index 18fb52c14acd7..ad3a23dc175de 100644
> --- a/arch/arm64/boot/dts/qcom/sm8750.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm8750.dtsi
> @@ -3005,6 +3005,209 @@ videocc: clock-controller@aaf0000 {
>  			#power-domain-cells = <1>;
>  		};
>  
> +		camss: isp@ad27000 {
> +			compatible = "qcom,sm8750-camss";

[Severity: Low]
Is there a dt-binding patch included in this series for the camss: isp@ad27000
node in arch/arm64/boot/dts/qcom/sm8750.dtsi? Adding the qcom,sm8750-camss
compatible string without matching documentation causes make dtbs_check to fail
when validating the new node.

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260618-pakala-camera-v2-0-9fbb729fd242@oss.qualcomm.com?part=1

^ permalink raw reply

* Re: [PATCH v5 2/2] hw_random: timeriomem-rng: add configurable read width and data mask
From: sashiko-bot @ 2026-06-18 12:13 UTC (permalink / raw)
  To: Jad Keskes; +Cc: conor+dt, devicetree, robh
In-Reply-To: <20260618120110.36439-2-inasj268@gmail.com>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Race condition and broken teardown sequence between module unbind and active hwrng readers causes Deadlocks or Use-After-Free of the private data.
--

commit d12f91b70dc751fd43c31e87b02671ebcad42270
Author: Jad Keskes <inasj268@gmail.com>

hw_random: timeriomem-rng: add configurable read width and data mask

This commit implements configurable read sizes and data masking for the
timeriomem-rng driver. It adds support for 1, 2, or 4-byte reads and an
optional bitmask to filter output entropy, configurable via platform data
or device tree.

> diff --git a/drivers/char/hw_random/timeriomem-rng.c b/drivers/char/hw_random/timeriomem-rng.c
> index e61f063932090..42393409f22ad 100644
> --- a/drivers/char/hw_random/timeriomem-rng.c
> +++ b/drivers/char/hw_random/timeriomem-rng.c

[ ... ]

> @@ -71,11 +76,28 @@ static int timeriomem_rng_read(struct hwrng *hwrng, void *data,
>  			usleep_range(period_us,
>  					period_us + max(1, period_us / 100));
>  
> -		*(u32 *)data = readl(priv->io_base);
> -		retval += sizeof(u32);
> -		data += sizeof(u32);
> -		max -= sizeof(u32);
> -	} while (wait && max > sizeof(u32));
> +		switch (priv->reg_io_width) {
> +		case 1: {
> +			u8 val = readb(priv->io_base) & priv->mask;
> +			*(u8 *)data = val;
> +			break;
> +		}
> +		case 2: {
> +			u16 val = readw(priv->io_base) & priv->mask;
> +			*(u16 *)data = val;
> +			break;
> +		}
> +		case 4: {
> +			u32 val = readl(priv->io_base) & priv->mask;
> +			*(u32 *)data = val;
> +			break;
> +		}
> +		}
> +
> +		retval += chunk;
> +		data += chunk;
> +		max -= chunk;
> +	} while (wait && max > chunk);
>  
>  	/*
>  	 * Block any new callers until the RNG has had time to generate new

[Severity: High]
This is a pre-existing issue, but can this sequence lead to deadlocks or a 
use-after-free during module teardown?

Because the driver uses devm_hwrng_register(), devres unregisters the RNG
after the driver's remove function returns. The timeriomem_rng_remove function
explicitly calls hrtimer_cancel(&priv->timer), but concurrent readers might
still be active in timeriomem_rng_read.

If a reader is waiting earlier in this function at
wait_for_completion(&priv->completion) when the timer is cancelled, won't it
hang forever since the timer will never fire to wake it up?

Alternatively, if the reader is already past the wait, it will unconditionally
call hrtimer_restart(&priv->timer) slightly further down at the end of this
function. Doesn't this re-arm the timer after it was cancelled by the removal
thread?

If the timer is re-armed and devres subsequently frees the
timeriomem_rng_private data, won't the timer eventually fire and access freed
memory?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260618120110.36439-1-inasj268@gmail.com?part=2

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox