mbox series

[PATCHv4,00/13] Introduce RK806 Support

Message ID 20221020204251.108565-1-sebastian.reichel@collabora.com
Headers show
Series Introduce RK806 Support | expand

Message

Sebastian Reichel Oct. 20, 2022, 8:42 p.m. UTC
Hi,

The Rockchip RK3588 Evaluation Boards use SPI connected RK806
PMICs. Downstream this is handled by a new driver, but apart
from being SPI connected this chip is quite similar to the
ther Rockchip PMICs (also RK806 is promoted to also support
I2C). Thus this series instead updates the RK808 driver(s).

Changelog since PATCHv3:
 * https://lore.kernel.org/all/20220909175522.179175-1-sebastian.reichel@collabora.com/
 * Dropped removing REGMAP_I2C dependency from RK817 ASoC driver (applied)
 * Rename MFD_RK808 to MFD_RK8XX to be consistent. It makes sense to do this now,
   since the patchset touches all the child drivers anyways.
 * rebase to v6.1-rc1
 * collected a couple of Acks
 * update rk806 DT binding according to DT maintainer feedback
 * add missing pinmux config to the rk806 DT binding
 * update rk806_spi_bus_write and rk806_spi_bus_read
 * replaced some constants with sizeof or defines
 * used capitalized comments
 * rename regmap_find_closest_bigger to regulator_find_closest_bigger, not sure
   why I prefixed it with regmap_ in the first place
 * use rk8xx_is_enabled_wmsk_regmap instead of regulator_is_enabled_regmap for
   the switching regulators to correctly report the state
 * reordered the first few patches grouping the MFD patches together

Changelog since PATCHv2:
 * https://lore.kernel.org/all/20220908003107.220143-1-sebastian.reichel@collabora.com/
 * Change DT binding to not allow nldo-reg6
 * Fix DT binding to check for [np]ldo-reg instead of [np]ldo_reg
 * remove rk806_get_voltage_sel_regmap in favour of regulator_get_voltage_sel_regmap
 * drop rk806_set_voltage in favour of regulator_set_voltage_sel_regmap
 * use regulator_set_ramp_delay_regmap
 * drop possibly incorrect printing of chip id register address in case of errors

Changelog since PATCHv1:
 * https://lore.kernel.org/all/20220831215437.117880-1-sebastian.reichel@collabora.com/
 * Collect Acked-by
 * Avoid if/else checks for regulator id in rk806 regulator driver
 * Fix indentation in DTS example section of the rk806 binding
 * Use absolute path for regulator.yaml referencing in the rk806 binding
 * Reduce pattern for DCDC regulators to only allow 1-10
 * replace uppercase name with lowercase ones in regulator names
 * replace _ with - in regulator names

-- Sebastian

Sebastian Reichel (13):
  clk: RK808: reduce 'struct rk808' usage
  regulator: rk808: reduce 'struct rk808' usage
  rtc: rk808: reduce 'struct rk808' usage
  mfd: rk808: convert to device managed resources
  mfd: rk808: use dev_err_probe
  mfd: rk808: replace 'struct i2c_client' with 'struct device'
  mfd: rk808: split into core and i2c
  dt-bindings: mfd: add rk806 binding
  mfd: rk8xx: add rk806 support
  pinctrl: rk805: add rk806 pinctrl support
  regulator: rk808: Use dev_err_probe
  regulator: expose regulator_find_closest_bigger
  regulator: rk808: add rk806 support

 .../bindings/mfd/rockchip,rk806.yaml          | 405 +++++++++++++++++
 drivers/clk/Kconfig                           |   2 +-
 drivers/clk/clk-rk808.c                       |  34 +-
 drivers/input/misc/Kconfig                    |   2 +-
 drivers/mfd/Kconfig                           |  21 +-
 drivers/mfd/Makefile                          |   4 +-
 drivers/mfd/{rk808.c => rk8xx-core.c}         | 351 +++++----------
 drivers/mfd/rk8xx-i2c.c                       | 209 +++++++++
 drivers/mfd/rk8xx-spi.c                       | 115 +++++
 drivers/pinctrl/Kconfig                       |   2 +-
 drivers/pinctrl/pinctrl-rk805.c               | 189 +++++++-
 drivers/power/supply/Kconfig                  |   2 +-
 drivers/regulator/Kconfig                     |   2 +-
 drivers/regulator/helpers.c                   |  22 +-
 drivers/regulator/rk808-regulator.c           | 409 ++++++++++++++++-
 drivers/rtc/Kconfig                           |   2 +-
 drivers/rtc/rtc-rk808.c                       |  47 +-
 include/linux/mfd/rk808.h                     | 417 +++++++++++++++++-
 include/linux/regulator/driver.h              |   2 +
 sound/soc/codecs/Kconfig                      |   2 +-
 20 files changed, 1913 insertions(+), 326 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mfd/rockchip,rk806.yaml
 rename drivers/mfd/{rk808.c => rk8xx-core.c} (70%)
 create mode 100644 drivers/mfd/rk8xx-i2c.c
 create mode 100644 drivers/mfd/rk8xx-spi.c

Comments

Rob Herring Oct. 21, 2022, 10:19 p.m. UTC | #1
On Thu, Oct 20, 2022 at 10:42:46PM +0200, Sebastian Reichel wrote:
> Add DT binding document for Rockchip's RK806 PMIC.
> 
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> ---
>  .../bindings/mfd/rockchip,rk806.yaml          | 405 ++++++++++++++++++
>  1 file changed, 405 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mfd/rockchip,rk806.yaml
> 
> diff --git a/Documentation/devicetree/bindings/mfd/rockchip,rk806.yaml b/Documentation/devicetree/bindings/mfd/rockchip,rk806.yaml
> new file mode 100644
> index 000000000000..4e907dd1f7a4
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/rockchip,rk806.yaml
> @@ -0,0 +1,405 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mfd/rockchip,rk806.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: RK806 Power Management Integrated Circuit
> +
> +maintainers:
> +  - Sebastian Reichel <sebastian.reichel@collabora.com>
> +
> +description: |

Don't need '|' if no formatting to preserve.

> +  Rockchip RK806 series PMIC. This device consists of an spi or
> +  i2c controlled MFD that includes multiple switchable regulators.
> +
> +properties:
> +  compatible:
> +    enum:
> +      - rockchip,rk806
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  gpio-controller: true
> +
> +  '#gpio-cells':
> +    const: 2
> +
> +  vcc1-supply:
> +    description:
> +      The input supply for dcdc-reg1.
> +
> +  vcc2-supply:
> +    description:
> +      The input supply for dcdc-reg2.
> +
> +  vcc3-supply:
> +    description:
> +      The input supply for dcdc-reg3.
> +
> +  vcc4-supply:
> +    description:
> +      The input supply for dcdc-reg4.
> +
> +  vcc5-supply:
> +    description:
> +      The input supply for dcdc-reg5.
> +
> +  vcc6-supply:
> +    description:
> +      The input supply for dcdc-reg6.
> +
> +  vcc7-supply:
> +    description:
> +      The input supply for dcdc-reg7.
> +
> +  vcc8-supply:
> +    description:
> +      The input supply for dcdc-reg8.
> +
> +  vcc9-supply:
> +    description:
> +      The input supply for dcdc-reg9.
> +
> +  vcc10-supply:
> +    description:
> +      The input supply for dcdc-reg10.
> +
> +  vcc11-supply:
> +    description:
> +      The input supply for pldo-reg1, pldo-reg2 and pldo-reg3.
> +
> +  vcc12-supply:
> +    description:
> +      The input supply for pldo-reg4 and pldo-reg5.
> +
> +  vcc13-supply:
> +    description:
> +      The input supply for nldo-reg1, nldo-reg2 and nldo-reg3.
> +
> +  vcc14-supply:
> +    description:
> +      The input supply for nldo-reg4 and nldo-reg5.
> +
> +  vcca-supply:
> +    description:
> +      The input supply for pldo-reg6.
> +
> +  regulators:
> +    type: object
> +    patternProperties:
> +      "^(dcdc-reg([1-9]|10)|pldo-reg[1-6]|nldo-reg[1-5])$":
> +        type: object
> +        $ref: /schemas/regulator/regulator.yaml#
> +        unevaluatedProperties: false
> +    additionalProperties: false

In the indented cases, this is a bit easier to read if placed before 
patternProperties.

> +
> +patternProperties:
> +  '-pins$':
> +    type: object

       additionalProperties: false

(or unevaluatedProperties if other properties besides function and pins 
apply.)

> +    $ref: /schemas/pinctrl/pinmux-node.yaml
> +
> +    properties:
> +      function:
> +        enum: [pin_fun0, pin_fun1, pin_fun2, pin_fun3, pin_fun4, pin_fun5]
> +
> +      pins:
> +        $ref: "/schemas/types.yaml#/definitions/string"

Can drop quotes.

> +        enum: [gpio_pwrctrl1, gpio_pwrctrl2, gpio_pwrctrl3]
> +
> +allOf:
> +  - $ref: /schemas/spi/spi-peripheral-props.yaml
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/pinctrl/rockchip.h>
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    #include <dt-bindings/gpio/gpio.h>
> +    spi {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        pmic@0 {
> +            compatible = "rockchip,rk806";
> +            reg = <0x0>;
> +
> +            interrupts = <7 IRQ_TYPE_LEVEL_LOW>;
> +
> +            vcc1-supply = <&vcc5v0_sys>;
> +            vcc2-supply = <&vcc5v0_sys>;
> +            vcc3-supply = <&vcc5v0_sys>;
> +            vcc4-supply = <&vcc5v0_sys>;
> +            vcc5-supply = <&vcc5v0_sys>;
> +            vcc6-supply = <&vcc5v0_sys>;
> +            vcc7-supply = <&vcc5v0_sys>;
> +            vcc8-supply = <&vcc5v0_sys>;
> +            vcc9-supply = <&vcc5v0_sys>;
> +            vcc10-supply = <&vcc5v0_sys>;
> +            vcc11-supply = <&vcc_2v0_pldo_s3>;
> +            vcc12-supply = <&vcc5v0_sys>;
> +            vcc13-supply = <&vcc5v0_sys>;
> +            vcc14-supply = <&vcc_1v1_nldo_s3>;
> +            vcca-supply = <&vcc5v0_sys>;
> +
> +            regulators {
> +                vdd_gpu_s0: dcdc-reg1 {
> +                    regulator-always-on;
> +                    regulator-boot-on;
> +                    regulator-min-microvolt = <550000>;
> +                    regulator-max-microvolt = <950000>;
> +                    regulator-ramp-delay = <12500>;
> +                    regulator-name = "vdd_gpu_s0";
> +                    regulator-state-mem {
> +                        regulator-off-in-suspend;
> +                    };
> +                };
> +
> +                vdd_npu_s0: dcdc-reg2 {
> +                    regulator-always-on;
> +                    regulator-boot-on;
> +                    regulator-min-microvolt = <550000>;
> +                    regulator-max-microvolt = <950000>;
> +                    regulator-ramp-delay = <12500>;
> +                    regulator-name = "vdd_npu_s0";
> +                    regulator-state-mem {
> +                        regulator-off-in-suspend;
> +                    };
> +                };
> +
> +                vdd_log_s0: dcdc-reg3 {
> +                    regulator-always-on;
> +                    regulator-boot-on;
> +                    regulator-min-microvolt = <750000>;
> +                    regulator-max-microvolt = <750000>;
> +                    regulator-ramp-delay = <12500>;
> +                    regulator-name = "vdd_log_s0";
> +                    regulator-state-mem {
> +                        regulator-on-in-suspend;
> +                        regulator-suspend-microvolt = <750000>;
> +                    };
> +                };
> +
> +                vdd_vdenc_s0: dcdc-reg4 {
> +                    regulator-always-on;
> +                    regulator-boot-on;
> +                    regulator-min-microvolt = <550000>;
> +                    regulator-max-microvolt = <950000>;
> +                    regulator-ramp-delay = <12500>;
> +                    regulator-name = "vdd_vdenc_s0";
> +                    regulator-state-mem {
> +                        regulator-off-in-suspend;
> +                    };
> +                };
> +
> +                vdd_gpu_mem_s0: dcdc-reg5 {
> +                    regulator-always-on;
> +                    regulator-boot-on;
> +                    regulator-min-microvolt = <675000>;
> +                    regulator-max-microvolt = <950000>;
> +                    regulator-ramp-delay = <12500>;
> +                    regulator-name = "vdd_gpu_mem_s0";
> +                    regulator-state-mem {
> +                        regulator-off-in-suspend;
> +                    };
> +                };
> +
> +                vdd_npu_mem_s0: dcdc-reg6 {
> +                    regulator-always-on;
> +                    regulator-boot-on;
> +                    regulator-min-microvolt = <675000>;
> +                    regulator-max-microvolt = <950000>;
> +                    regulator-ramp-delay = <12500>;
> +                    regulator-name = "vdd_npu_mem_s0";
> +                    regulator-state-mem {
> +                        regulator-off-in-suspend;
> +                    };
> +                };
> +
> +                vcc_2v0_pldo_s3: dcdc-reg7 {
> +                    regulator-always-on;
> +                    regulator-boot-on;
> +                    regulator-min-microvolt = <2000000>;
> +                    regulator-max-microvolt = <2000000>;
> +                    regulator-ramp-delay = <12500>;
> +                    regulator-name = "vdd_2v0_pldo_s3";
> +                    regulator-state-mem {
> +                        regulator-on-in-suspend;
> +                        regulator-suspend-microvolt = <2000000>;
> +                    };
> +                };
> +
> +                vdd_vdenc_mem_s0: dcdc-reg8 {
> +                    regulator-always-on;
> +                    regulator-boot-on;
> +                    regulator-min-microvolt = <675000>;
> +                    regulator-max-microvolt = <950000>;
> +                    regulator-ramp-delay = <12500>;
> +                    regulator-name = "vdd_vdenc_mem_s0";
> +                    regulator-state-mem {
> +                        regulator-off-in-suspend;
> +                    };
> +                };
> +
> +                vdd2_ddr_s3: dcdc-reg9 {
> +                    regulator-always-on;
> +                    regulator-boot-on;
> +                    regulator-name = "vdd2_ddr_s3";
> +                    regulator-state-mem {
> +                        regulator-on-in-suspend;
> +                    };
> +                };
> +
> +                vcc_1v1_nldo_s3: dcdc-reg10 {
> +                    regulator-always-on;
> +                    regulator-boot-on;
> +                    regulator-min-microvolt = <1100000>;
> +                    regulator-max-microvolt = <1100000>;
> +                    regulator-ramp-delay = <12500>;
> +                    regulator-name = "vcc_1v1_nldo_s3";
> +                    regulator-state-mem {
> +                        regulator-on-in-suspend;
> +                        regulator-suspend-microvolt = <1100000>;
> +                    };
> +                };
> +
> +                avcc_1v8_s0: pldo-reg1 {
> +                    regulator-always-on;
> +                    regulator-boot-on;
> +                    regulator-min-microvolt = <1800000>;
> +                    regulator-max-microvolt = <1800000>;
> +                    regulator-ramp-delay = <12500>;
> +                    regulator-name = "avcc_1v8_s0";
> +                    regulator-state-mem {
> +                        regulator-off-in-suspend;
> +                    };
> +                };
> +
> +                vdd1_1v8_ddr_s3: pldo-reg2 {
> +                    regulator-always-on;
> +                    regulator-boot-on;
> +                    regulator-min-microvolt = <1800000>;
> +                    regulator-max-microvolt = <1800000>;
> +                    regulator-ramp-delay = <12500>;
> +                    regulator-name = "vdd1_1v8_ddr_s3";
> +                    regulator-state-mem {
> +                        regulator-on-in-suspend;
> +                        regulator-suspend-microvolt = <1800000>;
> +                    };
> +                };
> +
> +                vcc_1v8_s3: pldo-reg3 {
> +                    regulator-always-on;
> +                    regulator-boot-on;
> +                    regulator-min-microvolt = <1800000>;
> +                    regulator-max-microvolt = <1800000>;
> +                    regulator-ramp-delay = <12500>;
> +                    regulator-name = "vcc_1v8_s3";
> +                    regulator-state-mem {
> +                        regulator-on-in-suspend;
> +                        regulator-suspend-microvolt = <1800000>;
> +                    };
> +                };
> +
> +                vcc_3v3_s0: pldo-reg4 {
> +                    regulator-always-on;
> +                    regulator-boot-on;
> +                    regulator-min-microvolt = <3300000>;
> +                    regulator-max-microvolt = <3300000>;
> +                    regulator-ramp-delay = <12500>;
> +                    regulator-name = "vcc_3v3_s0";
> +                    regulator-state-mem {
> +                        regulator-off-in-suspend;
> +                    };
> +                };
> +
> +                vccio_sd_s0: pldo-reg5 {
> +                    regulator-always-on;
> +                    regulator-boot-on;
> +                    regulator-min-microvolt = <1800000>;
> +                    regulator-max-microvolt = <3300000>;
> +                    regulator-ramp-delay = <12500>;
> +                    regulator-name = "vccio_sd_s0";
> +                    regulator-state-mem {
> +                        regulator-off-in-suspend;
> +                    };
> +                };
> +
> +                master_pldo6_s3: pldo-reg6 {
> +                    regulator-always-on;
> +                    regulator-boot-on;
> +                    regulator-min-microvolt = <1800000>;
> +                    regulator-max-microvolt = <1800000>;
> +                    regulator-name = "master_pldo6_s3";
> +                    regulator-state-mem {
> +                        regulator-on-in-suspend;
> +                        regulator-suspend-microvolt = <1800000>;
> +                    };
> +                };
> +
> +                vdd_0v75_s3: nldo-reg1 {
> +                    regulator-always-on;
> +                    regulator-boot-on;
> +                    regulator-min-microvolt = <750000>;
> +                    regulator-max-microvolt = <750000>;
> +                    regulator-ramp-delay = <12500>;
> +                    regulator-name = "vdd_0v75_s3";
> +                    regulator-state-mem {
> +                        regulator-on-in-suspend;
> +                        regulator-suspend-microvolt = <750000>;
> +                    };
> +                };
> +
> +                vdd2l_0v9_ddr_s3: nldo-reg2 {
> +                    regulator-always-on;
> +                    regulator-boot-on;
> +                    regulator-min-microvolt = <900000>;
> +                    regulator-max-microvolt = <900000>;
> +                    regulator-name = "vdd2l_0v9_ddr_s3";
> +                    regulator-state-mem {
> +                        regulator-on-in-suspend;
> +                        regulator-suspend-microvolt = <900000>;
> +                    };
> +                };
> +
> +                master_nldo3: nldo-reg3 {
> +                    regulator-name = "master_nldo3";
> +                    regulator-state-mem {
> +                        regulator-off-in-suspend;
> +                    };
> +                };
> +
> +                avdd_0v75_s0: nldo-reg4 {
> +                    regulator-always-on;
> +                    regulator-boot-on;
> +                    regulator-min-microvolt = <750000>;
> +                    regulator-max-microvolt = <750000>;
> +                    regulator-name = "avdd_0v75_s0";
> +                    regulator-state-mem {
> +                        regulator-off-in-suspend;
> +                    };
> +                };
> +
> +                vdd_0v85_s0: nldo-reg5 {
> +                    regulator-always-on;
> +                    regulator-boot-on;
> +                    regulator-min-microvolt = <850000>;
> +                    regulator-max-microvolt = <850000>;
> +                    regulator-name = "vdd_0v85_s0";
> +                    regulator-state-mem {
> +                        regulator-off-in-suspend;
> +                    };
> +                };
> +            };
> +        };
> +    };
> -- 
> 2.35.1
> 
>
Sebastian Reichel Nov. 25, 2022, 11:46 p.m. UTC | #2
Hi Mark,

On Fri, Nov 25, 2022 at 07:33:32PM +0000, Mark Brown wrote:
> On Thu, Oct 20, 2022 at 10:42:49PM +0200, Sebastian Reichel wrote:
> > Print error message for potential EPROBE_DEFER error using
> > dev_err_probe, which captures the reason in
> > /sys/kernel/debug/devices_deferred and otherwise silences
> > the message.
> 
> This doesn't apply against current code, please check and resend.

That's expected. Patch 2 changes the dev parameter of dev_err(),
which is being changed to dev_err_probe() in this patch.

I think we are getting too close to the merge window for this
series considering it's touching so many subsystems. But you
can take patch 2 + 11 to the regulators tree as is.

Thanks for the review,

-- Sebastian
Sebastian Reichel Nov. 25, 2022, 11:52 p.m. UTC | #3
Hi Alexandre,

On Thu, Oct 20, 2022 at 10:42:41PM +0200, Sebastian Reichel wrote:
> Reduce usage of 'struct rk808' (driver data of the parent MFD), so
> that only the chip variant field is still being accessed directly.
> This allows restructuring the MFD driver to support SPI based
> PMICs.
> 
> Acked-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> ---

You can take this patch. I don't think it makes sense to send
another version of this series before the merge window considering
it involves so many subystems. If the RTC patch is merged now it
does not need to become part of the immutable branch later :)

Thanks,

-- Sebastian

>  drivers/rtc/rtc-rk808.c | 47 ++++++++++++++++++-----------------------
>  1 file changed, 20 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/rtc/rtc-rk808.c b/drivers/rtc/rtc-rk808.c
> index e920da8c08da..2d9bcb3ce1e3 100644
> --- a/drivers/rtc/rtc-rk808.c
> +++ b/drivers/rtc/rtc-rk808.c
> @@ -14,7 +14,6 @@
>  #include <linux/bcd.h>
>  #include <linux/mfd/rk808.h>
>  #include <linux/platform_device.h>
> -#include <linux/i2c.h>
>  
>  /* RTC_CTRL_REG bitfields */
>  #define BIT_RTC_CTRL_REG_STOP_RTC_M		BIT(0)
> @@ -51,7 +50,7 @@ struct rk_rtc_compat_reg {
>  };
>  
>  struct rk808_rtc {
> -	struct rk808 *rk808;
> +	struct regmap *regmap;
>  	struct rtc_device *rtc;
>  	struct rk_rtc_compat_reg *creg;
>  	int irq;
> @@ -97,12 +96,11 @@ static void gregorian_to_rockchip(struct rtc_time *tm)
>  static int rk808_rtc_readtime(struct device *dev, struct rtc_time *tm)
>  {
>  	struct rk808_rtc *rk808_rtc = dev_get_drvdata(dev);
> -	struct rk808 *rk808 = rk808_rtc->rk808;
>  	u8 rtc_data[NUM_TIME_REGS];
>  	int ret;
>  
>  	/* Force an update of the shadowed registers right now */
> -	ret = regmap_update_bits(rk808->regmap, rk808_rtc->creg->ctrl_reg,
> +	ret = regmap_update_bits(rk808_rtc->regmap, rk808_rtc->creg->ctrl_reg,
>  				 BIT_RTC_CTRL_REG_RTC_GET_TIME,
>  				 BIT_RTC_CTRL_REG_RTC_GET_TIME);
>  	if (ret) {
> @@ -116,7 +114,7 @@ static int rk808_rtc_readtime(struct device *dev, struct rtc_time *tm)
>  	 * 32khz. If we clear the GET_TIME bit here, the time of i2c transfer
>  	 * certainly more than 31.25us: 16 * 2.5us at 400kHz bus frequency.
>  	 */
> -	ret = regmap_update_bits(rk808->regmap, rk808_rtc->creg->ctrl_reg,
> +	ret = regmap_update_bits(rk808_rtc->regmap, rk808_rtc->creg->ctrl_reg,
>  				 BIT_RTC_CTRL_REG_RTC_GET_TIME,
>  				 0);
>  	if (ret) {
> @@ -124,7 +122,7 @@ static int rk808_rtc_readtime(struct device *dev, struct rtc_time *tm)
>  		return ret;
>  	}
>  
> -	ret = regmap_bulk_read(rk808->regmap, rk808_rtc->creg->seconds_reg,
> +	ret = regmap_bulk_read(rk808_rtc->regmap, rk808_rtc->creg->seconds_reg,
>  			       rtc_data, NUM_TIME_REGS);
>  	if (ret) {
>  		dev_err(dev, "Failed to bulk read rtc_data: %d\n", ret);
> @@ -148,7 +146,6 @@ static int rk808_rtc_readtime(struct device *dev, struct rtc_time *tm)
>  static int rk808_rtc_set_time(struct device *dev, struct rtc_time *tm)
>  {
>  	struct rk808_rtc *rk808_rtc = dev_get_drvdata(dev);
> -	struct rk808 *rk808 = rk808_rtc->rk808;
>  	u8 rtc_data[NUM_TIME_REGS];
>  	int ret;
>  
> @@ -163,7 +160,7 @@ static int rk808_rtc_set_time(struct device *dev, struct rtc_time *tm)
>  	rtc_data[6] = bin2bcd(tm->tm_wday);
>  
>  	/* Stop RTC while updating the RTC registers */
> -	ret = regmap_update_bits(rk808->regmap, rk808_rtc->creg->ctrl_reg,
> +	ret = regmap_update_bits(rk808_rtc->regmap, rk808_rtc->creg->ctrl_reg,
>  				 BIT_RTC_CTRL_REG_STOP_RTC_M,
>  				 BIT_RTC_CTRL_REG_STOP_RTC_M);
>  	if (ret) {
> @@ -171,14 +168,14 @@ static int rk808_rtc_set_time(struct device *dev, struct rtc_time *tm)
>  		return ret;
>  	}
>  
> -	ret = regmap_bulk_write(rk808->regmap, rk808_rtc->creg->seconds_reg,
> +	ret = regmap_bulk_write(rk808_rtc->regmap, rk808_rtc->creg->seconds_reg,
>  				rtc_data, NUM_TIME_REGS);
>  	if (ret) {
>  		dev_err(dev, "Failed to bull write rtc_data: %d\n", ret);
>  		return ret;
>  	}
>  	/* Start RTC again */
> -	ret = regmap_update_bits(rk808->regmap, rk808_rtc->creg->ctrl_reg,
> +	ret = regmap_update_bits(rk808_rtc->regmap, rk808_rtc->creg->ctrl_reg,
>  				 BIT_RTC_CTRL_REG_STOP_RTC_M, 0);
>  	if (ret) {
>  		dev_err(dev, "Failed to update RTC control: %d\n", ret);
> @@ -191,12 +188,11 @@ static int rk808_rtc_set_time(struct device *dev, struct rtc_time *tm)
>  static int rk808_rtc_readalarm(struct device *dev, struct rtc_wkalrm *alrm)
>  {
>  	struct rk808_rtc *rk808_rtc = dev_get_drvdata(dev);
> -	struct rk808 *rk808 = rk808_rtc->rk808;
>  	u8 alrm_data[NUM_ALARM_REGS];
>  	uint32_t int_reg;
>  	int ret;
>  
> -	ret = regmap_bulk_read(rk808->regmap,
> +	ret = regmap_bulk_read(rk808_rtc->regmap,
>  			       rk808_rtc->creg->alarm_seconds_reg,
>  			       alrm_data, NUM_ALARM_REGS);
>  	if (ret) {
> @@ -212,7 +208,7 @@ static int rk808_rtc_readalarm(struct device *dev, struct rtc_wkalrm *alrm)
>  	alrm->time.tm_year = (bcd2bin(alrm_data[5] & YEARS_REG_MSK)) + 100;
>  	rockchip_to_gregorian(&alrm->time);
>  
> -	ret = regmap_read(rk808->regmap, rk808_rtc->creg->int_reg, &int_reg);
> +	ret = regmap_read(rk808_rtc->regmap, rk808_rtc->creg->int_reg, &int_reg);
>  	if (ret) {
>  		dev_err(dev, "Failed to read RTC INT REG: %d\n", ret);
>  		return ret;
> @@ -228,10 +224,9 @@ static int rk808_rtc_readalarm(struct device *dev, struct rtc_wkalrm *alrm)
>  
>  static int rk808_rtc_stop_alarm(struct rk808_rtc *rk808_rtc)
>  {
> -	struct rk808 *rk808 = rk808_rtc->rk808;
>  	int ret;
>  
> -	ret = regmap_update_bits(rk808->regmap, rk808_rtc->creg->int_reg,
> +	ret = regmap_update_bits(rk808_rtc->regmap, rk808_rtc->creg->int_reg,
>  				 BIT_RTC_INTERRUPTS_REG_IT_ALARM_M, 0);
>  
>  	return ret;
> @@ -239,10 +234,9 @@ static int rk808_rtc_stop_alarm(struct rk808_rtc *rk808_rtc)
>  
>  static int rk808_rtc_start_alarm(struct rk808_rtc *rk808_rtc)
>  {
> -	struct rk808 *rk808 = rk808_rtc->rk808;
>  	int ret;
>  
> -	ret = regmap_update_bits(rk808->regmap, rk808_rtc->creg->int_reg,
> +	ret = regmap_update_bits(rk808_rtc->regmap, rk808_rtc->creg->int_reg,
>  				 BIT_RTC_INTERRUPTS_REG_IT_ALARM_M,
>  				 BIT_RTC_INTERRUPTS_REG_IT_ALARM_M);
>  
> @@ -252,7 +246,6 @@ static int rk808_rtc_start_alarm(struct rk808_rtc *rk808_rtc)
>  static int rk808_rtc_setalarm(struct device *dev, struct rtc_wkalrm *alrm)
>  {
>  	struct rk808_rtc *rk808_rtc = dev_get_drvdata(dev);
> -	struct rk808 *rk808 = rk808_rtc->rk808;
>  	u8 alrm_data[NUM_ALARM_REGS];
>  	int ret;
>  
> @@ -272,7 +265,7 @@ static int rk808_rtc_setalarm(struct device *dev, struct rtc_wkalrm *alrm)
>  	alrm_data[4] = bin2bcd(alrm->time.tm_mon + 1);
>  	alrm_data[5] = bin2bcd(alrm->time.tm_year - 100);
>  
> -	ret = regmap_bulk_write(rk808->regmap,
> +	ret = regmap_bulk_write(rk808_rtc->regmap,
>  				rk808_rtc->creg->alarm_seconds_reg,
>  				alrm_data, NUM_ALARM_REGS);
>  	if (ret) {
> @@ -313,20 +306,18 @@ static int rk808_rtc_alarm_irq_enable(struct device *dev,
>  static irqreturn_t rk808_alarm_irq(int irq, void *data)
>  {
>  	struct rk808_rtc *rk808_rtc = data;
> -	struct rk808 *rk808 = rk808_rtc->rk808;
> -	struct i2c_client *client = rk808->i2c;
>  	int ret;
>  
> -	ret = regmap_write(rk808->regmap, rk808_rtc->creg->status_reg,
> +	ret = regmap_write(rk808_rtc->regmap, rk808_rtc->creg->status_reg,
>  			   RTC_STATUS_MASK);
>  	if (ret) {
> -		dev_err(&client->dev,
> +		dev_err(&rk808_rtc->rtc->dev,
>  			"%s:Failed to update RTC status: %d\n", __func__, ret);
>  		return ret;
>  	}
>  
>  	rtc_update_irq(rk808_rtc->rtc, 1, RTC_IRQF | RTC_AF);
> -	dev_dbg(&client->dev,
> +	dev_dbg(&rk808_rtc->rtc->dev,
>  		 "%s:irq=%d\n", __func__, irq);
>  	return IRQ_HANDLED;
>  }
> @@ -404,10 +395,12 @@ static int rk808_rtc_probe(struct platform_device *pdev)
>  		break;
>  	}
>  	platform_set_drvdata(pdev, rk808_rtc);
> -	rk808_rtc->rk808 = rk808;
> +	rk808_rtc->regmap = dev_get_regmap(pdev->dev.parent, NULL);
> +	if (!rk808_rtc->regmap)
> +		return -ENODEV;
>  
>  	/* start rtc running by default, and use shadowed timer. */
> -	ret = regmap_update_bits(rk808->regmap, rk808_rtc->creg->ctrl_reg,
> +	ret = regmap_update_bits(rk808_rtc->regmap, rk808_rtc->creg->ctrl_reg,
>  				 BIT_RTC_CTRL_REG_STOP_RTC_M |
>  				 BIT_RTC_CTRL_REG_RTC_READSEL_M,
>  				 BIT_RTC_CTRL_REG_RTC_READSEL_M);
> @@ -417,7 +410,7 @@ static int rk808_rtc_probe(struct platform_device *pdev)
>  		return ret;
>  	}
>  
> -	ret = regmap_write(rk808->regmap, rk808_rtc->creg->status_reg,
> +	ret = regmap_write(rk808_rtc->regmap, rk808_rtc->creg->status_reg,
>  			   RTC_STATUS_MASK);
>  	if (ret) {
>  		dev_err(&pdev->dev,
> -- 
> 2.35.1
>
Mark Brown Nov. 28, 2022, 7:05 p.m. UTC | #4
On Thu, 20 Oct 2022 22:42:38 +0200, Sebastian Reichel wrote:
> The Rockchip RK3588 Evaluation Boards use SPI connected RK806
> PMICs. Downstream this is handled by a new driver, but apart
> from being SPI connected this chip is quite similar to the
> ther Rockchip PMICs (also RK806 is promoted to also support
> I2C). Thus this series instead updates the RK808 driver(s).
> 
> Changelog since PATCHv3:
>  * https://lore.kernel.org/all/20220909175522.179175-1-sebastian.reichel@collabora.com/
>  * Dropped removing REGMAP_I2C dependency from RK817 ASoC driver (applied)
>  * Rename MFD_RK808 to MFD_RK8XX to be consistent. It makes sense to do this now,
>    since the patchset touches all the child drivers anyways.
>  * rebase to v6.1-rc1
>  * collected a couple of Acks
>  * update rk806 DT binding according to DT maintainer feedback
>  * add missing pinmux config to the rk806 DT binding
>  * update rk806_spi_bus_write and rk806_spi_bus_read
>  * replaced some constants with sizeof or defines
>  * used capitalized comments
>  * rename regmap_find_closest_bigger to regulator_find_closest_bigger, not sure
>    why I prefixed it with regmap_ in the first place
>  * use rk8xx_is_enabled_wmsk_regmap instead of regulator_is_enabled_regmap for
>    the switching regulators to correctly report the state
>  * reordered the first few patches grouping the MFD patches together
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git for-next

Thanks!

[02/13] regulator: rk808: reduce 'struct rk808' usage
        commit: 647e57351f8ebc37d8e12cbc0f4bf7471754a0cc
[11/13] regulator: rk808: Use dev_err_probe
        commit: f39f8709c217d82aabbf51d8669731137ce09aea

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark
Alexandre Belloni Dec. 11, 2022, 7:59 p.m. UTC | #5
On Thu, 20 Oct 2022 22:42:38 +0200, Sebastian Reichel wrote:
> The Rockchip RK3588 Evaluation Boards use SPI connected RK806
> PMICs. Downstream this is handled by a new driver, but apart
> from being SPI connected this chip is quite similar to the
> ther Rockchip PMICs (also RK806 is promoted to also support
> I2C). Thus this series instead updates the RK808 driver(s).
> 
> Changelog since PATCHv3:
>  * https://lore.kernel.org/all/20220909175522.179175-1-sebastian.reichel@collabora.com/
>  * Dropped removing REGMAP_I2C dependency from RK817 ASoC driver (applied)
>  * Rename MFD_RK808 to MFD_RK8XX to be consistent. It makes sense to do this now,
>    since the patchset touches all the child drivers anyways.
>  * rebase to v6.1-rc1
>  * collected a couple of Acks
>  * update rk806 DT binding according to DT maintainer feedback
>  * add missing pinmux config to the rk806 DT binding
>  * update rk806_spi_bus_write and rk806_spi_bus_read
>  * replaced some constants with sizeof or defines
>  * used capitalized comments
>  * rename regmap_find_closest_bigger to regulator_find_closest_bigger, not sure
>    why I prefixed it with regmap_ in the first place
>  * use rk8xx_is_enabled_wmsk_regmap instead of regulator_is_enabled_regmap for
>    the switching regulators to correctly report the state
>  * reordered the first few patches grouping the MFD patches together
> 
> [...]

Applied, thanks!

[03/13] rtc: rk808: reduce 'struct rk808' usage
        commit: 2e830ccc21eb67a4c2490279d907e5e9199e5156

Best regards,