regulator: rtmv20: Adds support for Richtek RTMV20 load switch regulator
Subsystems (2)
PATCH 1/2 regulator: rtmv20: Adds support for Richtek RTMV20 load switch regulator
From: ChiYuan Huang <[email protected]>
Add support for Richtek RTMV20 load switch regulator.
--- drivers/regulator/Kconfig | 9 + drivers/regulator/Makefile | 1 + drivers/regulator/rtmv20-regulator.c | 512 +++++++++++++++++++++++++++++++++++ 3 files changed, 522 insertions(+) create mode 100644 drivers/regulator/rtmv20-regulator.c
drivers/regulator/Kconfig +9 -0
@@ -902,6 +902,15 @@ config REGULATOR_RT5033 RT5033 PMIC. The device supports multiple regulators like current source, LDO and Buck. +config REGULATOR_RTMV20 + tristate "RTMV20 Laser Diode Regulator" + depends on I2C + select REGMAP_I2C + help + This driver adds support for the load switch current regulator on + the Richtek RTMV20. It can support the load current up to 6A and + integrate strobe/vsync/fsin signal to synchronize the IR camera. + config REGULATOR_S2MPA01 tristate "Samsung S2MPA01 voltage regulator" depends on MFD_SEC_CORE
drivers/regulator/Makefile +1 -0
@@ -112,6 +112,7 @@ obj-$(CONFIG_REGULATOR_RK808) += rk808-regulator.o obj-$(CONFIG_REGULATOR_RN5T618) += rn5t618-regulator.o obj-$(CONFIG_REGULATOR_ROHM) += rohm-regulator.o obj-$(CONFIG_REGULATOR_RT5033) += rt5033-regulator.o +obj-$(CONFIG_REGULATOR_RTMV20) += rtmv20-regulator.o obj-$(CONFIG_REGULATOR_S2MPA01) += s2mpa01.o obj-$(CONFIG_REGULATOR_S2MPS11) += s2mps11.o obj-$(CONFIG_REGULATOR_S5M8767) += s5m8767.o
drivers/regulator/rtmv20-regulator.c new file +512 -0
512 lines not on this page · raw message
Signature · 1 line
2.7.4
PATCH 2/2 regulator: rtmv20: Add DT-binding document for Richtek RTMV20
From: ChiYuan Huang <[email protected]>
Add DT-binding document for Richtek RTMV20
--- .../regulator/richtek,rtmv20-regulator.yaml | 183 +++++++++++++++++++++ 1 file changed, 183 insertions(+) create mode 100644 Documentation/devicetree/bindings/regulator/richtek,rtmv20-regulator.yaml
Documentation/devicetree/bindings/regulator/richtek,rtmv20-regulator.yaml new file +183 -0
@@ -0,0 +1,183 @@ +# SPDX-License-Identifier: GPL-2.0 +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/regulator/richtek,rtmv20-regulator.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Richtek RTMV20 laser diode regulator + +maintainers: + - ChiYuan Huang <[email protected]> + +description: | + Richtek RTMV20 is a load switch current regulator that can supply up to 6A. + It is used to drive laser diode. There're two signals for chip controls + (Enable/Fail), Enable pin to turn chip on, and Fail pin as fault indication. + There're still four pins for camera control, two inputs (strobe and vsync), + the others for outputs (fsin1 and fsin2). Strobe input to start the current + supply, vsync input from IR camera, and fsin1/fsin2 output for the optional. + +properties: + compatible: + const: richtek,rtmv20 + + reg: + maxItems: 1 + + wakeup-source: true + + interrupts-extend: + maxItems: 1 + + enable-gpio: + description: A connection of the 'enable' gpio line. + maxItems: 1 + + ld_pulse_delay: + description: | + load current pulse delay in microsecond after strobe pin pulse high. + $ref: "/schemas/types.yaml#/definitions/uint32" + minimum: 0 + maximum: 100000 + default: 0 + + ld_pulse_width: + description: | + Load current pulse width in microsecond after strobe pin pulse high. + $ref: "/schemas/types.yaml#/definitions/uint32" + minimum: 0 + maximum: 10000 + default: 1200 + + fsin1_delay: + description: | + Fsin1 pulse high delay in microsecond after vsync signal pulse high. + $ref: "/schemas/types.yaml#/definitions/uint32" + minimum: 0 + maximum: 100000 + default: 23000 + + fsin1_width: + description: | + Fsin1 pulse high width in microsecond after vsync signal pulse high. + $ref: "/schemas/types.yaml#/definitions/uint32" + minimum: 40 + maximum: 10000 + default: 160 + + fsin2_delay: + description: | + Fsin2 pulse high delay in microsecond after vsync signal pulse high. + $ref: "/schemas/types.yaml#/definitions/uint32" + minimum: 0 + maximum: 100000 + default: 23000 + + fsin2_width: + description: | + Fsin2 pulse high width in microsecond after vsync signal pulse high. + $ref: "/schemas/types.yaml#/definitions/uint32" + minimum: 40 + maximum: 10000 + default: 160 + + es_pulse_width: + description: Eye safety function pulse width limit in microsecond. + $ref: "/schemas/types.yaml#/definitions/uint32" + minimum: 0 + maximum: 10000 + default: 1200 + + es_ld_current: + description: Eye safety function load current limit in microamp. + $ref: "/schemas/types.yaml#/definitions/uint32" + minimum: 0 + maximum: 6000000 + default: 3000000 + + lbp_level: + description: Low battery protection level in microvolt. + $ref: "/schemas/types.yaml#/definitions/uint32" + minimum: 2400000 + maximum: 3700000 + default: 2700000 + + lbp_enable: + description: Low battery protection function enable control. + $ref: "/schemas/types.yaml#/definitions/uint32" + minimum: 0 + maximum: 1 + default: 0 + + strobe_polarity: + description: Strobe pin active polarity control. + $ref: "/schemas/types.yaml#/definitions/uint32" + minimum: 0 + maximum: 1 + default: 1 + + vsync_polarity: + description: Vsync pin active polarity control. + $ref: "/schemas/types.yaml#/definitions/uint32" + minimum: 0 + maximum: 1 + default: 1 + + fsin_enable: + description: Fsin function enable control. + $ref: "/schemas/types.yaml#/definitions/uint32" + minimum: 0 + maximum: 1 + default: 0 + + fsin_output: + description: Fsin function output control. + $ref: "/schemas/types.yaml#/definitions/uint32" + minimum: 0 + maximum: 1 + default: 0 + + es_enable: + description: Eye safety function enable control. + $ref: "/schemas/types.yaml#/definitions/uint32" + minimum: 0 + maximum: 1 + default: 0 + +patternProperties: + "lsw": + type: object + $ref: "regulator.yaml#" + +required: + - compatible + - reg + - wakeup-source + - interrupts-extend + - enable-gpio + - lsw + +additionalProperties: false + +examples: + - | + #include <dt-bindings/interrupt-controller/irq.h> + i2c { + #address-cells = <1>; + #size-cells = <0>; + + rtmv20@34 { + compatible = "richtek,rtmv20"; + reg = <0x34>; + wakeup-source; + interrupts-extend = <&gpio26 2 IRQ_TYPE_LEVEL_LOW>; + enable-gpio = <&gpio26 3 0>; + + lsw { + regulator-name = "rtmv20,lsw"; + regulator-min-microamp = <0>; + regulator-max-microamp = <6000000>; + }; + }; + }; +...
Signature · 1 line
2.7.4
Mark Brown
maintainerRe: [PATCH v1 2/2] regulator: rtmv20: Add DT-binding document for Richtek RTMV20
Documentation/devicetree/bindings/regulator/richtek,rtmv20-regulator.yaml line 34
Quoted 3 lines · cy_huang
Documentation/devicetree/bindings/regulator/richtek,rtmv20-regulator.yaml new file +3
-0+ enable-gpio: + description: A connection of the 'enable' gpio line. + maxItems: 1
-gpios. GPIO properties should always be plural even if there's only one GPIO.
Quoted 1 line
+
ld_pulse_delay:
Properties should use - not _ and for all the properties specifying things like times you should have units so...
Documentation/devicetree/bindings/regulator/richtek,rtmv20-regulator.yaml line 39
Quoted 3 lines
Documentation/devicetree/bindings/regulator/richtek,rtmv20-regulator.yaml new file +3
-0+ description: | + load current pulse delay in microsecond after strobe pin pulse high. + $ref: "/schemas/types.yaml#/definitions/uint32"
...add -us here.
Documentation/devicetree/bindings/regulator/richtek,rtmv20-regulator.yaml line 131
Quoted 6 lines
Documentation/devicetree/bindings/regulator/richtek,rtmv20-regulator.yaml new file +6
-0+ fsin_enable: + description: Fsin function enable control. + $ref: "/schemas/types.yaml#/definitions/uint32" + minimum: 0 + maximum: 1 + default: 0
This looks like it should be a boolean property not a number. The same is true for most if not all of the other properties with min/max of 0/1.
A message that is not in the archive (sent off-list or not yet received): CADiBU39ShyHNT=NiWz2Y81+wLMJjDqD6aZ0mgzVSxp_rzLZSFQ@mail.gmail.com
Re: [PATCH v1 2/2] regulator: rtmv20: Add DT-binding document for Richtek RTMV20
Re-add into the mail loop list.
ChiYuan Huang <[email protected]> 於 2020年9月25日 週五 下午12:09寫道：
Quoted 34 lines
Mark Brown <[email protected]> 於 2020年9月25日 週五 上午12:24寫道：
Quoted 12 lines · cy_huang
Documentation/devicetree/bindings/regulator/richtek,rtmv20-regulator.yaml line 34
Quoted 4 lines
Documentation/devicetree/bindings/regulator/richtek,rtmv20-regulator.yaml new file +3
-0+ enable-gpio: + description: A connection of the 'enable' gpio line. + maxItems: 1-gpios. GPIO properties should always be plural even if there's only one GPIO.
Quoted 1 line
+
ld_pulse_delay:Quoted 9 lines
Properties should use - not _ and for all the properties specifying things like times you should have units so...
Documentation/devicetree/bindings/regulator/richtek,rtmv20-regulator.yaml line 39
Quoted 3 lines
Documentation/devicetree/bindings/regulator/richtek,rtmv20-regulator.yaml new file +3
-0+ description: | + load current pulse delay in microsecond after strobe pin pulse high. + $ref: "/schemas/types.yaml#/definitions/uint32"...add -us here.
Ack
Quoted 9 lines
Documentation/devicetree/bindings/regulator/richtek,rtmv20-regulator.yaml line 131
Quoted 6 lines
Documentation/devicetree/bindings/regulator/richtek,rtmv20-regulator.yaml new file +6
-0+ fsin_enable: + description: Fsin function enable control. + $ref: "/schemas/types.yaml#/definitions/uint32" + minimum: 0 + maximum: 1 + default: 0This looks like it should be a boolean property not a number. The same is true for most if not all of the other properties with min/max of 0/1.
Ok, just change the property as boolean type.
Mark Brown
maintainerdrivers/regulator/rtmv20-regulator.c line 105
Quoted 9 lines · cy_huang
drivers/regulator/rtmv20-regulator.c new file +9
-0+static int rtmv20_apply_properties (struct rtmv20_priv *priv) +{ +const struct { +int offset; +int len; +unsigned int reg; +unsigned int mask; + } props[] = { +PROP_REG_DECL (ld_pulse_delay,RTMV20_REG_PULSEDELAY ,RTMV20_DELAY_MASK ),
I can't help but feel that this looks like a register cache would be a simpler way of achieving this.
drivers/regulator/rtmv20-regulator.c line 143
Quoted 3 lines
drivers/regulator/rtmv20-regulator.c new file +3
-0+switch (props[i].len) { +case RTMV20_2BYTE_ACCES : +case RTMV20_1BYTE_ACCES :
It feels like this should all be abstracted in the regmap with custom reg_read() and reg_write() operations - or have two regmaps for the two different register sizes. Having the register sizes and endianness handling outside of regmap code seems like an abstraction failure.
drivers/regulator/rtmv20-regulator.c line 181
Quoted 5 lines
drivers/regulator/rtmv20-regulator.c new file +5
-0+/* If chip is not enabled, only kept the value, instead */ +if (!priv->is_chip_enabled) +return 0 ; + +return regmap_update_bits (priv->regmap, rdev->desc->csel_reg, rdev->desc->csel_mask, sel);
This sort of stuff works a lot better with a register cache.
drivers/regulator/rtmv20-regulator.c line 216
Quoted 10 lines
drivers/regulator/rtmv20-regulator.c new file +10
-0+static int rtmv20_lsw_enable (struct regulator_dev *rdev) +{ +struct rtmv20_priv *priv =rdev_get_drvdata (rdev); +int ret; + +gpiod_set_value (priv->enable_gpio,1 ); +/* Wait for I2C can be accessed */ +usleep_range (RTMV20_I2CRDY_TIMEUS ,RTMV20_I2CRDY_TIMEUS +100 ); + +/* If enable gpio from low to high, the whole registers will be reset, applied here */
Please keep to 80 columns.
Quoted 7 lines
drivers/regulator/rtmv20-regulator.c new file +7
-0+ ret =regmap_read (priv->regmap,RTMV20_REG_LDIRQ , &val); +if (ret) { +dev_err (priv->dev,"Failed to get irq flags\n" ); +return IRQ_NONE ; + } + +if (val &OTPEVT_MASK )
drivers/regulator/rtmv20-regulator.c line 310
Quoted 10 lines
drivers/regulator/rtmv20-regulator.c new file +10
-0+regulator_notifier_call_chain (priv->rdev,REGULATOR_EVENT_OVER_TEMP ,NULL ); + +if (val &OCPEVT_MASK ) +regulator_notifier_call_chain (priv->rdev,REGULATOR_EVENT_OVER_CURRENT ,NULL ); + +if (val &FAILEVT_MASK ) +regulator_notifier_call_chain (priv->rdev,REGULATOR_EVENT_FAIL ,NULL ); + +return IRQ_HANDLED ; +}
It looks like this is clear on read but shouln't there be at least some display of unknown values?
drivers/regulator/rtmv20-regulator.c line 447
Quoted 2 lines
drivers/regulator/rtmv20-regulator.c new file +2
-0+/* After chip check, keep in shutdown mode for low quiescent current */ +gpiod_set_value (priv->enable_gpio,0 );
Don't do this, the driver should not adjust the system state unless explicitly told to do so - we don't know if any state changes are safe on a given board.
A message that is not in the archive (sent off-list or not yet received): CADiBU3_b69g9vZf-J8p2_m5xDjn24XaUHn88foBsQ4MpBN829g@mail.gmail.com
A message that is not in the archive (sent off-list or not yet received): [email protected]
Mark Brown <[email protected]> 於 2020年9月25日 週五 下午7:33寫道：
Quoted 8 lines · ChiYuan Huang
Quoted 1 line
Mark Brown <[email protected]> 於 2020年9月25日 週五 上午12:12寫道：
Please don't take things off-list unless there is a really strong reason to do so. Sending things to the list ensures that everyone gets a chance to read and comment on things.
Sorry, I press the wrong button to only reply one not reply all. Loop in the mail list again.
Quoted 11 lines
Quoted 10 lines
Quoted 2 lines
I can't help but feel that this looks like a register cache would be a simpler way of achieving this.
This is because of enable gpio limitation. If gpio low to high, all registers will be reset.
And enable gpio high to low will cause I2C cannot be accessed. That's why we need to apply register setting after hardware enable pin be pulled high.
The consumption current for EN L vs H is also different from 1uA vs 450uA defined as typical values.
Quoted 4 lines
That's exactly the sort of situation that regmap caches are usually used for, mark the device as cache only when powering off then resync after power on.
Thx, I think I catch the point.
Quoted 9 lines
Quoted 8 lines
Quoted 8 lines
drivers/regulator/rtmv20-regulator.c line 143
Quoted 3 lines
drivers/regulator/rtmv20-regulator.c new file +3
-0+switch (props[i].len) { +case RTMV20_2BYTE_ACCES : +case RTMV20_1BYTE_ACCES :It feels like this should all be abstracted in the regmap with custom
reg_read()andreg_write()operations - or have two regmaps for the two different register sizes. Having the register sizes and endianness handling outside of regmap code seems like an abstraction failure.
Quoted 6 lines
Quoted 3 lines
Actually, it's the consecutive register address with two byte data. Lower address defined as
val_h, and the higher defined asval_l. So I just usecpu_to_be16to do the transformation.Oh, so just a straight regmap would be fine.
Thx.
Quoted 5 lines
Quoted 2 lines
From the above hardware description, do you have any suggestion to achieve the register cache?
Look at how other devices use
regcache_cache_only().
Thx.
Quoted 4 lines
Quoted 3 lines
Quoted 3 lines
Don't do this, the driver should not adjust the system state unless explicitly told to do so - we don't know if any state changes are safe on a given board.
Quoted 8 lines
Quoted 5 lines
From my original coding, it's put before regulator register. After regulator registered, it will follow the regulator framework to do the turn on/off and the other settings.
I think it won't affect the system state, just to keep IC as shutdown mode (1uA) after chip vendor info check.It depends if the device was enabled before the driver started, and if anything started powering on when the device was powered on.
Sure, I'll re-check the flow.
kernel test robot
build botThank you for the patch! Perhaps something to improve:
Hi cy_huang,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on v5.9-rc6]
[also build test WARNING on next-20200924]
[cannot apply to regulator/for-next]
[If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/cy_huang/regulator-rtmv20-Adds-support-for-Richtek-RTMV20-load-switch-regulator/20200924-230631 base: ba4f184e126b751d1bffad5897f263108befc780 config: m68k-randconfig-s031-20200925 (attached as .config) compiler: m68k-linux-gcc (GCC) 9.3.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# apt-get install sparse
# sparse version: v0.6.2-201-g24bdaac6-dirty
# https://github.com/0day-ci/linux/commit/98eb1836ead59206f951ad8466fde2faca6472f4
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review cy_huang/regulator-rtmv20-Adds-support-for-Richtek-RTMV20-load-switch-regulator/20200924-230631 git checkout 98eb1836ead59206f951ad8466fde2faca6472f4
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=m68k
If you fix the issue, kindly add following tag as appropriate
sparse warnings: (new ones prefixed by >>)
>> drivers/regulator/rtmv20-regulator.c:136:31: sparse: sparse: cast to restricted __be16 >> drivers/regulator/rtmv20-regulator.c:139:29: sparse: sparse: incorrect type in assignment (different base types) @@ expected unsigned short [addressable] [assigned] [usertype] tmp @@ got restricted __be16 [usertype] @@ >> drivers/regulator/rtmv20-regulator.c:139:29: sparse: expected unsigned short [addressable] [assigned] [usertype] tmp >> drivers/regulator/rtmv20-regulator.c:139:29: sparse: got restricted __be16 [usertype]
vim +136 drivers/regulator/rtmv20-regulator.c
96
97 static int rtmv20_apply_properties(struct rtmv20_priv *priv)
98 {
99 const struct {
100 int offset;
101 int len;
102 unsigned int reg;
103 unsigned int mask;
104 } props[] = {
105 PROP_REG_DECL(ld_pulse_delay, RTMV20_REG_PULSEDELAY, RTMV20_DELAY_MASK),
106 PROP_REG_DECL(ld_pulse_width, RTMV20_REG_PULSEWIDTH, RTMV20_WIDTH_MASK),
107 PROP_REG_DECL(fsin1_delay, RTMV20_REG_FSIN1CTRL1, RTMV20_DELAY_MASK),
108 PROP_REG_DECL(fsin1_width, RTMV20_REG_FSIN1CTRL3, RTMV20_WIDTH2_MASK),
109 PROP_REG_DECL(fsin2_delay, RTMV20_REG_FSIN2CTRL1, RTMV20_DELAY_MASK),
110 PROP_REG_DECL(fsin2_width, RTMV20_REG_FSIN2CTRL3, RTMV20_WIDTH2_MASK),
111 PROP_REG_DECL(es_pulse_width, RTMV20_REG_ESPULSEWIDTH, RTMV20_WIDTH_MASK),
112 PROP_REG_DECL(es_ld_current, RTMV20_REG_ESLDCTRL1, RTMV20_LDCURR_MASK),
113 PROP_REG_DECL(lbp_level, RTMV20_REG_LBP, RTMV20_LBPLVL_MASK),
114 PROP_REG_DECL(lbp_enable, RTMV20_REG_LBP, RTMV20_LBPEN_MASK),
115 PROP_REG_DECL(strobe_polarity, RTMV20_REG_LDCTRL2, RTMV20_STROBEPOL_MASK),
116 PROP_REG_DECL(vsync_polarity, RTMV20_REG_LDCTRL2, RTMV20_VSYNPOL_MASK),
117 PROP_REG_DECL(fsin_enable, RTMV20_REG_ENCTRL, RTMV20_FSINEN_MASK),
118 PROP_REG_DECL(fsin_output, RTMV20_REG_ENCTRL, RTMV20_FSINOUT_MASK),
119 PROP_REG_DECL(es_enable, RTMV20_REG_ENCTRL, RTMV20_ESEN_MASK),
120 };
121 void *start = &priv->properties;
122 int i;
123
124 for (i = 0; i < ARRAY_SIZE(props); i++) {
125 u16 tmp = 0;
126 u16 *u16p = start + props[i].offset;
127 u8 *u8p = start + props[i].offset;
128 int shift = ffs(props[i].mask) - 1, ret;
129
130 switch (props[i].len) {
131 case RTMV20_2BYTE_ACCES:
132 ret = regmap_raw_read(priv->regmap, props[i].reg, &tmp, props[i].len);
133 if (ret)
134 return ret;
135
> 136 tmp = be16_to_cpu(tmp);
137 tmp &= ~props[i].mask;
138 tmp |= (*u16p << shift);
> 139 tmp = cpu_to_be16(tmp); 140
141 ret = regmap_raw_write(priv->regmap, props[i].reg, &tmp, props[i].len);
142 break;
143 case RTMV20_1BYTE_ACCES:
144 ret = regmap_update_bits(priv->regmap, props[i].reg, props[i].mask,
145 *u8p << shift);
146 break;
147 default:
148 return -EINVAL;
149 }
150
151 if (ret)
152 return ret;
153 }
154
155 return 0;
156 }
157
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]