* [PATCH 0/2] RTQ6056: Add compatible for the same chip family
@ 2023-12-28 3:51 cy_huang
2023-12-28 3:51 ` [PATCH 1/2] dt-bindings: iio: adc: rtq6056: add support for the whole RTQ6056 family cy_huang
` (2 more replies)
0 siblings, 3 replies; 17+ messages in thread
From: cy_huang @ 2023-12-28 3:51 UTC (permalink / raw)
To: Jonathan Cameron, Krzysztof Kozlowski, Conor Dooley
Cc: Lars-Peter Clausen, Rob Herring, ChiYuan Huang,
Uwe Kleine-König, linux-iio, devicetree, linux-kernel
From: ChiYuan Huang <cy_huang@richtek.com>
*** Resend for the '--thread' config in 'git format-patch'.
RTQ6053 and RTQ6059 are the same RTQ6056 family.
The differences are listed below
- RTQ6053
Only change chip package type
- RTQ6059
1. Enlarge the shunt voltage sensing range
2. Shrink the pinout for VBUS sense pin
3. Due to 1, the scale value is also changed
ChiYuan Huang (2):
dt-bindings: iio: adc: rtq6056: add support for the whole RTQ6056
family
iio: adc: rtq6056: Add support for the whole RTQ6056 family
.../bindings/iio/adc/richtek,rtq6056.yaml | 5 +-
drivers/iio/adc/rtq6056.c | 265 +++++++++++++++++-
2 files changed, 255 insertions(+), 15 deletions(-)
--
2.34.1
^ permalink raw reply [flat|nested] 17+ messages in thread* [PATCH 1/2] dt-bindings: iio: adc: rtq6056: add support for the whole RTQ6056 family 2023-12-28 3:51 [PATCH 0/2] RTQ6056: Add compatible for the same chip family cy_huang @ 2023-12-28 3:51 ` cy_huang 2023-12-28 7:15 ` Krzysztof Kozlowski 2023-12-28 3:51 ` [PATCH 2/2] iio: adc: rtq6056: Add " cy_huang 2023-12-28 7:16 ` [PATCH 0/2] RTQ6056: Add compatible for the same chip family Krzysztof Kozlowski 2 siblings, 1 reply; 17+ messages in thread From: cy_huang @ 2023-12-28 3:51 UTC (permalink / raw) To: Jonathan Cameron, Krzysztof Kozlowski, Conor Dooley Cc: Lars-Peter Clausen, Rob Herring, ChiYuan Huang, Uwe Kleine-König, linux-iio, devicetree, linux-kernel From: ChiYuan Huang <cy_huang@richtek.com> Add compatible support for RTQ6053 and RTQ6059. Signed-off-by: ChiYuan Huang <cy_huang@richtek.com> --- .../devicetree/bindings/iio/adc/richtek,rtq6056.yaml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/Documentation/devicetree/bindings/iio/adc/richtek,rtq6056.yaml b/Documentation/devicetree/bindings/iio/adc/richtek,rtq6056.yaml index 88e008629ea8..d1e1f36d1972 100644 --- a/Documentation/devicetree/bindings/iio/adc/richtek,rtq6056.yaml +++ b/Documentation/devicetree/bindings/iio/adc/richtek,rtq6056.yaml @@ -25,7 +25,10 @@ description: | properties: compatible: - const: richtek,rtq6056 + enum: + - richtek,rtq6053 + - richtek,rtq6056 + - richtek,rtq6059 reg: maxItems: 1 -- 2.34.1 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 1/2] dt-bindings: iio: adc: rtq6056: add support for the whole RTQ6056 family 2023-12-28 3:51 ` [PATCH 1/2] dt-bindings: iio: adc: rtq6056: add support for the whole RTQ6056 family cy_huang @ 2023-12-28 7:15 ` Krzysztof Kozlowski 0 siblings, 0 replies; 17+ messages in thread From: Krzysztof Kozlowski @ 2023-12-28 7:15 UTC (permalink / raw) To: cy_huang, Jonathan Cameron, Krzysztof Kozlowski, Conor Dooley Cc: Lars-Peter Clausen, Rob Herring, Uwe Kleine-König, linux-iio, devicetree, linux-kernel On 28/12/2023 04:51, cy_huang@richtek.com wrote: > From: ChiYuan Huang <cy_huang@richtek.com> > > Add compatible support for RTQ6053 and RTQ6059. > > Signed-off-by: ChiYuan Huang <cy_huang@richtek.com> Why do you resend the same patches after two days ignoring the feedback? No versioning, no changelog? NAK. This is a friendly reminder during the review process. It seems my or other reviewer's previous comments were not fully addressed. Maybe the feedback got lost between the quotes, maybe you just forgot to apply it. Please go back to the previous discussion and either implement all requested changes or keep discussing them. Thank you. Best regards, Krzysztof ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 2/2] iio: adc: rtq6056: Add support for the whole RTQ6056 family 2023-12-28 3:51 [PATCH 0/2] RTQ6056: Add compatible for the same chip family cy_huang 2023-12-28 3:51 ` [PATCH 1/2] dt-bindings: iio: adc: rtq6056: add support for the whole RTQ6056 family cy_huang @ 2023-12-28 3:51 ` cy_huang 2023-12-28 7:16 ` [PATCH 0/2] RTQ6056: Add compatible for the same chip family Krzysztof Kozlowski 2 siblings, 0 replies; 17+ messages in thread From: cy_huang @ 2023-12-28 3:51 UTC (permalink / raw) To: Jonathan Cameron, Krzysztof Kozlowski, Conor Dooley Cc: Lars-Peter Clausen, Rob Herring, ChiYuan Huang, Uwe Kleine-König, linux-iio, devicetree, linux-kernel From: ChiYuan Huang <cy_huang@richtek.com> RTQ6053 and RTQ6059 are the same series of RTQ6056. The respective differences with RTQ6056 are listed below RTQ6053 - chip package type RTQ6059 - Reduce the pinout for vbus sensing pin - Some internal ADC scaling change Signed-off-by: ChiYuan Huang <cy_huang@richtek.com> --- drivers/iio/adc/rtq6056.c | 265 ++++++++++++++++++++++++++++++++++++-- 1 file changed, 251 insertions(+), 14 deletions(-) diff --git a/drivers/iio/adc/rtq6056.c b/drivers/iio/adc/rtq6056.c index ad4cea6839b2..1bf43a90ca46 100644 --- a/drivers/iio/adc/rtq6056.c +++ b/drivers/iio/adc/rtq6056.c @@ -39,6 +39,16 @@ #define RTQ6056_DEFAULT_CONFIG 0x4127 #define RTQ6056_CONT_ALLON 7 +#define RTQ6059_DEFAULT_CONFIG 0x3C47 +#define RTQ6059_VBUS_LSB_OFFSET 3 +#define RTQ6059_AVG_BASE 8 + +enum { + RICHTEK_DEV_RTQ6056 = 0, + RICHTEK_DEV_RTQ6059, + RICHTEK_DEV_MAX +}; + enum { RTQ6056_CH_VSHUNT = 0, RTQ6056_CH_VBUS, @@ -50,16 +60,29 @@ enum { enum { F_OPMODE = 0, F_VSHUNTCT, + F_SADC = F_VSHUNTCT, F_VBUSCT, + F_BADC = F_VBUSCT, F_AVG, + F_PGA = F_AVG, F_RESET, F_MAX_FIELDS }; +struct richtek_dev_data { + int dev_id; + int default_conv_time; + unsigned int default_config; + unsigned int calib_coefficient; + const struct reg_field *reg_fields; + const struct iio_chan_spec *channels; +}; + struct rtq6056_priv { struct device *dev; struct regmap *regmap; struct regmap_field *rm_fields[F_MAX_FIELDS]; + const struct richtek_dev_data *devdata; u32 shunt_resistor_uohm; int vshuntct_us; int vbusct_us; @@ -74,6 +97,14 @@ static const struct reg_field rtq6056_reg_fields[F_MAX_FIELDS] = { [F_RESET] = REG_FIELD(RTQ6056_REG_CONFIG, 15, 15), }; +static const struct reg_field rtq6059_reg_fields[F_MAX_FIELDS] = { + [F_OPMODE] = REG_FIELD(RTQ6056_REG_CONFIG, 0, 2), + [F_SADC] = REG_FIELD(RTQ6056_REG_CONFIG, 3, 6), + [F_BADC] = REG_FIELD(RTQ6056_REG_CONFIG, 7, 10), + [F_PGA] = REG_FIELD(RTQ6056_REG_CONFIG, 11, 12), + [F_RESET] = REG_FIELD(RTQ6056_REG_CONFIG, 15, 15), +}; + static const struct iio_chan_spec rtq6056_channels[RTQ6056_MAX_CHANNEL + 1] = { { .type = IIO_VOLTAGE, @@ -151,10 +182,93 @@ static const struct iio_chan_spec rtq6056_channels[RTQ6056_MAX_CHANNEL + 1] = { IIO_CHAN_SOFT_TIMESTAMP(RTQ6056_MAX_CHANNEL), }; +/* + * Difference between RTQ6056 and RTQ6059 + * - Fixed sampling conversion time + * - Average sample numbers + * - Channel scale + * - calibration coefficient + */ +static const struct iio_chan_spec rtq6059_channels[RTQ6056_MAX_CHANNEL + 1] = { + { + .type = IIO_VOLTAGE, + .indexed = 1, + .channel = 0, + .address = RTQ6056_REG_SHUNTVOLT, + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | + BIT(IIO_CHAN_INFO_SCALE) | + BIT(IIO_CHAN_INFO_SAMP_FREQ), + .info_mask_shared_by_all = BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO), + .info_mask_shared_by_all_available = BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO), + .scan_index = 0, + .scan_type = { + .sign = 's', + .realbits = 16, + .storagebits = 16, + .endianness = IIO_CPU, + }, + }, + { + .type = IIO_VOLTAGE, + .indexed = 1, + .channel = 1, + .address = RTQ6056_REG_BUSVOLT, + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | + BIT(IIO_CHAN_INFO_SCALE) | + BIT(IIO_CHAN_INFO_SAMP_FREQ), + .info_mask_shared_by_all = BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO), + .info_mask_shared_by_all_available = BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO), + .scan_index = 1, + .scan_type = { + .sign = 'u', + .realbits = 16, + .storagebits = 16, + .endianness = IIO_CPU, + }, + }, + { + .type = IIO_POWER, + .indexed = 1, + .channel = 2, + .address = RTQ6056_REG_POWER, + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | + BIT(IIO_CHAN_INFO_SCALE) | + BIT(IIO_CHAN_INFO_SAMP_FREQ), + .info_mask_shared_by_all = BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO), + .info_mask_shared_by_all_available = BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO), + .scan_index = 2, + .scan_type = { + .sign = 'u', + .realbits = 16, + .storagebits = 16, + .endianness = IIO_CPU, + }, + }, + { + .type = IIO_CURRENT, + .indexed = 1, + .channel = 3, + .address = RTQ6056_REG_CURRENT, + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | + BIT(IIO_CHAN_INFO_SAMP_FREQ), + .info_mask_shared_by_all = BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO), + .info_mask_shared_by_all_available = BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO), + .scan_index = 3, + .scan_type = { + .sign = 's', + .realbits = 16, + .storagebits = 16, + .endianness = IIO_CPU, + }, + }, + IIO_CHAN_SOFT_TIMESTAMP(RTQ6056_MAX_CHANNEL), +}; + static int rtq6056_adc_read_channel(struct rtq6056_priv *priv, struct iio_chan_spec const *ch, int *val) { + const struct richtek_dev_data *devdata = priv->devdata; struct device *dev = priv->dev; unsigned int addr = ch->address; unsigned int regval; @@ -168,10 +282,18 @@ static int rtq6056_adc_read_channel(struct rtq6056_priv *priv, return ret; /* Power and VBUS is unsigned 16-bit, others are signed 16-bit */ - if (addr == RTQ6056_REG_BUSVOLT || addr == RTQ6056_REG_POWER) + switch (addr) { + case RTQ6056_REG_BUSVOLT: + if (devdata->dev_id == RICHTEK_DEV_RTQ6059) + regval >>= RTQ6059_VBUS_LSB_OFFSET; + fallthrough; + case RTQ6056_REG_POWER: *val = regval; - else + break; + default: *val = sign_extend32(regval, 16); + break; + } return IIO_VAL_INT; } @@ -199,6 +321,28 @@ static int rtq6056_adc_read_scale(struct iio_chan_spec const *ch, int *val, } } +static int rtq6059_adc_read_scale(struct iio_chan_spec const *ch, int *val, + int *val2) +{ + switch (ch->address) { + case RTQ6056_REG_SHUNTVOLT: + /* VSHUNT lsb 10uV */ + *val = 10000; + *val2 = 1000000; + return IIO_VAL_FRACTIONAL; + case RTQ6056_REG_BUSVOLT: + /* VBUS lsb 4mV */ + *val = 4; + return IIO_VAL_INT; + case RTQ6056_REG_POWER: + /* Power lsb 20mW */ + *val = 20; + return IIO_VAL_INT; + default: + return -EINVAL; + } +} + /* * Sample frequency for channel VSHUNT and VBUS. The indices correspond * with the bit value expected by the chip. And it can be found at @@ -248,6 +392,10 @@ static const int rtq6056_avg_sample_list[] = { 1, 4, 16, 64, 128, 256, 512, 1024, }; +static const int rtq6059_avg_sample_list[] = { + 1, 2, 4, 8, 16, 32, 64, 128, +}; + static int rtq6056_adc_set_average(struct rtq6056_priv *priv, int val) { unsigned int selector; @@ -268,6 +416,30 @@ static int rtq6056_adc_set_average(struct rtq6056_priv *priv, int val) return 0; } +static int rtq6059_adc_set_average(struct rtq6056_priv *priv, int val) +{ + unsigned int selector; + int ret; + + if (val > 128 || val < 1) + return -EINVAL; + + /* The supported average sample is 2^x (x from 0 to 7) */ + selector = fls(val) - 1; + + ret = regmap_field_write(priv->rm_fields[F_BADC], + RTQ6059_AVG_BASE + selector); + if (ret) + return ret; + + ret = regmap_field_write(priv->rm_fields[F_SADC], + RTQ6059_AVG_BASE + selector); + + priv->avg_sample = BIT(selector + 1); + + return 0; +} + static int rtq6056_adc_get_sample_freq(struct rtq6056_priv *priv, struct iio_chan_spec const *ch, int *val) { @@ -292,11 +464,15 @@ static int rtq6056_adc_read_raw(struct iio_dev *indio_dev, int *val2, long mask) { struct rtq6056_priv *priv = iio_priv(indio_dev); + const struct richtek_dev_data *devdata = priv->devdata; switch (mask) { case IIO_CHAN_INFO_RAW: return rtq6056_adc_read_channel(priv, chan, val); case IIO_CHAN_INFO_SCALE: + if (devdata->dev_id == RICHTEK_DEV_RTQ6059) + return rtq6059_adc_read_scale(chan, val, val2); + return rtq6056_adc_read_scale(chan, val, val2); case IIO_CHAN_INFO_OVERSAMPLING_RATIO: *val = priv->avg_sample; @@ -313,16 +489,28 @@ static int rtq6056_adc_read_avail(struct iio_dev *indio_dev, const int **vals, int *type, int *length, long mask) { + struct rtq6056_priv *priv = iio_priv(indio_dev); + const struct richtek_dev_data *devdata = priv->devdata; + switch (mask) { case IIO_CHAN_INFO_SAMP_FREQ: + if (devdata->dev_id == RICHTEK_DEV_RTQ6059) + return -EINVAL; + *vals = rtq6056_samp_freq_list; *type = IIO_VAL_INT; *length = ARRAY_SIZE(rtq6056_samp_freq_list); return IIO_AVAIL_LIST; case IIO_CHAN_INFO_OVERSAMPLING_RATIO: - *vals = rtq6056_avg_sample_list; + if (devdata->dev_id == RICHTEK_DEV_RTQ6059) { + *vals = rtq6059_avg_sample_list; + *length = ARRAY_SIZE(rtq6059_avg_sample_list); + } else { + *vals = rtq6056_avg_sample_list; + *length = ARRAY_SIZE(rtq6056_avg_sample_list); + } + *type = IIO_VAL_INT; - *length = ARRAY_SIZE(rtq6056_avg_sample_list); return IIO_AVAIL_LIST; default: return -EINVAL; @@ -334,6 +522,7 @@ static int rtq6056_adc_write_raw(struct iio_dev *indio_dev, int val2, long mask) { struct rtq6056_priv *priv = iio_priv(indio_dev); + const struct richtek_dev_data *devdata = priv->devdata; int ret; ret = iio_device_claim_direct_mode(indio_dev); @@ -342,10 +531,16 @@ static int rtq6056_adc_write_raw(struct iio_dev *indio_dev, switch (mask) { case IIO_CHAN_INFO_SAMP_FREQ: - ret = rtq6056_adc_set_samp_freq(priv, chan, val); + if (devdata->dev_id == RICHTEK_DEV_RTQ6059) + ret = -EINVAL; + else + ret = rtq6056_adc_set_samp_freq(priv, chan, val); break; case IIO_CHAN_INFO_OVERSAMPLING_RATIO: - ret = rtq6056_adc_set_average(priv, val); + if (devdata->dev_id == RICHTEK_DEV_RTQ6059) + ret = rtq6059_adc_set_average(priv, val); + else + ret = rtq6056_adc_set_average(priv, val); break; default: ret = -EINVAL; @@ -374,6 +569,7 @@ static int rtq6056_adc_read_label(struct iio_dev *indio_dev, static int rtq6056_set_shunt_resistor(struct rtq6056_priv *priv, int resistor_uohm) { + const struct richtek_dev_data *devdata = priv->devdata; unsigned int calib_val; int ret; @@ -382,8 +578,8 @@ static int rtq6056_set_shunt_resistor(struct rtq6056_priv *priv, return -EINVAL; } - /* calibration = 5120000 / (Rshunt (uOhm) * current lsb (1mA)) */ - calib_val = 5120000 / resistor_uohm; + /* calibration = coefficient / (Rshunt (uOhm) * current lsb (1mA)) */ + calib_val = devdata->calib_coefficient / resistor_uohm; ret = regmap_write(priv->regmap, RTQ6056_REG_CALIBRATION, calib_val); if (ret) return ret; @@ -445,11 +641,16 @@ static const struct iio_info rtq6056_info = { .read_label = rtq6056_adc_read_label, }; +static const struct iio_info rtq6059_info = { + .attrs = &rtq6056_attribute_group, +}; + static irqreturn_t rtq6056_buffer_trigger_handler(int irq, void *p) { struct iio_poll_func *pf = p; struct iio_dev *indio_dev = pf->indio_dev; struct rtq6056_priv *priv = iio_priv(indio_dev); + const struct richtek_dev_data *devdata = priv->devdata; struct device *dev = priv->dev; struct { u16 vals[RTQ6056_MAX_CHANNEL]; @@ -469,6 +670,10 @@ static irqreturn_t rtq6056_buffer_trigger_handler(int irq, void *p) if (ret) goto out; + if (devdata->dev_id == RICHTEK_DEV_RTQ6059 && + addr == RTQ6056_REG_BUSVOLT) + raw >>= RTQ6059_VBUS_LSB_OFFSET; + data.vals[i++] = raw; } @@ -528,20 +733,26 @@ static int rtq6056_probe(struct i2c_client *i2c) struct rtq6056_priv *priv; struct device *dev = &i2c->dev; struct regmap *regmap; + const struct richtek_dev_data *devdata; unsigned int vendor_id, shunt_resistor_uohm; int ret; if (!i2c_check_functionality(i2c->adapter, I2C_FUNC_SMBUS_WORD_DATA)) return -EOPNOTSUPP; + devdata = device_get_match_data(dev); + if (!devdata) + return dev_err_probe(dev, -EINVAL, "Invalid dev data\n"); + indio_dev = devm_iio_device_alloc(dev, sizeof(*priv)); if (!indio_dev) return -ENOMEM; priv = iio_priv(indio_dev); priv->dev = dev; - priv->vshuntct_us = priv->vbusct_us = 1037; + priv->vshuntct_us = priv->vbusct_us = devdata->default_conv_time; priv->avg_sample = 1; + priv->devdata = devdata; i2c_set_clientdata(i2c, priv); regmap = devm_regmap_init_i2c(i2c, &rtq6056_regmap_config); @@ -556,20 +767,26 @@ static int rtq6056_probe(struct i2c_client *i2c) return dev_err_probe(dev, ret, "Failed to get manufacturer info\n"); + /* For RTQ6059, this vendor id value is meaningless */ if (vendor_id != RTQ6056_VENDOR_ID) return dev_err_probe(dev, -ENODEV, "Invalid vendor id 0x%04x\n", vendor_id); ret = devm_regmap_field_bulk_alloc(dev, regmap, priv->rm_fields, - rtq6056_reg_fields, F_MAX_FIELDS); + devdata->reg_fields, F_MAX_FIELDS); if (ret) return dev_err_probe(dev, ret, "Failed to init regmap field\n"); /* + * RTQ6053 & RTQ6056: * By default, configure average sample as 1, bus and shunt conversion * time as 1037 microsecond, and operating mode to all on. + * + * RTQ6059: + * By default, configure average sample as 1, bus and shunt conversion + * time as 532 microsecond, and operating mode to all on. */ - ret = regmap_write(regmap, RTQ6056_REG_CONFIG, RTQ6056_DEFAULT_CONFIG); + ret = regmap_write(regmap, RTQ6056_REG_CONFIG, devdata->default_config); if (ret) return dev_err_probe(dev, ret, "Failed to enable continuous sensing\n"); @@ -598,8 +815,8 @@ static int rtq6056_probe(struct i2c_client *i2c) indio_dev->name = "rtq6056"; indio_dev->modes = INDIO_DIRECT_MODE; - indio_dev->channels = rtq6056_channels; - indio_dev->num_channels = ARRAY_SIZE(rtq6056_channels); + indio_dev->channels = devdata->channels; + indio_dev->num_channels = RTQ6056_MAX_CHANNEL + 1; indio_dev->info = &rtq6056_info; ret = devm_iio_triggered_buffer_setup(dev, indio_dev, NULL, @@ -640,8 +857,28 @@ static int rtq6056_runtime_resume(struct device *dev) static DEFINE_RUNTIME_DEV_PM_OPS(rtq6056_pm_ops, rtq6056_runtime_suspend, rtq6056_runtime_resume, NULL); +static const struct richtek_dev_data rtq6056_devdata = { + .dev_id = RICHTEK_DEV_RTQ6056, + .default_conv_time = 1037, + .calib_coefficient = 5120000, + .default_config = RTQ6056_DEFAULT_CONFIG, + .reg_fields = rtq6056_reg_fields, + .channels = rtq6056_channels, +}; + +static const struct richtek_dev_data rtq6059_devdata = { + .dev_id = RICHTEK_DEV_RTQ6059, + .default_conv_time = 532, + .calib_coefficient = 40960000, + .default_config = RTQ6059_DEFAULT_CONFIG, + .reg_fields = rtq6059_reg_fields, + .channels = rtq6059_channels, +}; + static const struct of_device_id rtq6056_device_match[] = { - { .compatible = "richtek,rtq6056" }, + { .compatible = "richtek,rtq6053", .data = &rtq6056_devdata }, + { .compatible = "richtek,rtq6056", .data = &rtq6056_devdata }, + { .compatible = "richtek,rtq6059", .data = &rtq6059_devdata }, {} }; MODULE_DEVICE_TABLE(of, rtq6056_device_match); -- 2.34.1 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 0/2] RTQ6056: Add compatible for the same chip family 2023-12-28 3:51 [PATCH 0/2] RTQ6056: Add compatible for the same chip family cy_huang 2023-12-28 3:51 ` [PATCH 1/2] dt-bindings: iio: adc: rtq6056: add support for the whole RTQ6056 family cy_huang 2023-12-28 3:51 ` [PATCH 2/2] iio: adc: rtq6056: Add " cy_huang @ 2023-12-28 7:16 ` Krzysztof Kozlowski 2023-12-28 7:27 ` ChiYuan Huang 2 siblings, 1 reply; 17+ messages in thread From: Krzysztof Kozlowski @ 2023-12-28 7:16 UTC (permalink / raw) To: cy_huang, Jonathan Cameron, Krzysztof Kozlowski, Conor Dooley Cc: Lars-Peter Clausen, Rob Herring, Uwe Kleine-König, linux-iio, devicetree, linux-kernel On 28/12/2023 04:51, cy_huang@richtek.com wrote: > From: ChiYuan Huang <cy_huang@richtek.com> > > *** Resend for the '--thread' config in 'git format-patch'. Resending the same after you receive feedback means you entirely bypass that feedback. Don't do this. Ever. Best regards, Krzysztof ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 0/2] RTQ6056: Add compatible for the same chip family 2023-12-28 7:16 ` [PATCH 0/2] RTQ6056: Add compatible for the same chip family Krzysztof Kozlowski @ 2023-12-28 7:27 ` ChiYuan Huang 0 siblings, 0 replies; 17+ messages in thread From: ChiYuan Huang @ 2023-12-28 7:27 UTC (permalink / raw) To: Krzysztof Kozlowski Cc: Jonathan Cameron, Krzysztof Kozlowski, Conor Dooley, Lars-Peter Clausen, Rob Herring, Uwe Kleine-König, linux-iio, devicetree, linux-kernel On Thu, Dec 28, 2023 at 08:16:40AM +0100, Krzysztof Kozlowski wrote: > On 28/12/2023 04:51, cy_huang@richtek.com wrote: > > From: ChiYuan Huang <cy_huang@richtek.com> > > > > *** Resend for the '--thread' config in 'git format-patch'. > > Resending the same after you receive feedback means you entirely bypass > that feedback. Don't do this. Ever. > Got it. I'm just trying to figure out why Jonathan said the thread cannot be found in lore.kernel.org. Eventually I found that's the 'thread' option. When sending v2 patch series, I think I won't forget it in the future. Not inteneded to bypass the whole suggestion or feedback. Sorry for that. > Best regards, > Krzysztof > ^ permalink raw reply [flat|nested] 17+ messages in thread
[parent not found: <1703562468-29052-1-git-send-email-cy_huang@richtek.com>]
[parent not found: <1703562468-29052-2-git-send-email-cy_huang@richtek.com>]
* Re: [PATCH 1/2] dt-bindings: iio: adc: rtq6056: add support for the whole RTQ6056 family [not found] ` <1703562468-29052-2-git-send-email-cy_huang@richtek.com> @ 2023-12-26 9:18 ` Krzysztof Kozlowski 2023-12-26 11:19 ` ChiYuan Huang 2023-12-26 16:24 ` Jonathan Cameron 0 siblings, 2 replies; 17+ messages in thread From: Krzysztof Kozlowski @ 2023-12-26 9:18 UTC (permalink / raw) To: cy_huang, Jonathan Cameron, Krzysztof Kozlowski, Conor Dooley Cc: Lars-Peter Clausen, Rob Herring, Uwe Kleine-König, linux-iio, devicetree, linux-kernel On 26/12/2023 04:47, cy_huang@richtek.com wrote: > From: ChiYuan Huang <cy_huang@richtek.com> > > Add compatible support for RTQ6053 and RTQ6059. > > Signed-off-by: ChiYuan Huang <cy_huang@richtek.com> > --- > .../devicetree/bindings/iio/adc/richtek,rtq6056.yaml | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/Documentation/devicetree/bindings/iio/adc/richtek,rtq6056.yaml b/Documentation/devicetree/bindings/iio/adc/richtek,rtq6056.yaml > index 88e008629ea8..d1e1f36d1972 100644 > --- a/Documentation/devicetree/bindings/iio/adc/richtek,rtq6056.yaml > +++ b/Documentation/devicetree/bindings/iio/adc/richtek,rtq6056.yaml > @@ -25,7 +25,10 @@ description: | > > properties: > compatible: > - const: richtek,rtq6056 > + enum: > + - richtek,rtq6053 > + - richtek,rtq6056 Aren't these devices compatible? Your driver change says they are, so express compatibility with list here (and oneOf). Best regards, Krzysztof ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/2] dt-bindings: iio: adc: rtq6056: add support for the whole RTQ6056 family 2023-12-26 9:18 ` [PATCH 1/2] dt-bindings: iio: adc: rtq6056: add support for the whole RTQ6056 family Krzysztof Kozlowski @ 2023-12-26 11:19 ` ChiYuan Huang 2023-12-26 12:12 ` Krzysztof Kozlowski 2023-12-26 16:24 ` Jonathan Cameron 1 sibling, 1 reply; 17+ messages in thread From: ChiYuan Huang @ 2023-12-26 11:19 UTC (permalink / raw) To: Krzysztof Kozlowski Cc: Jonathan Cameron, Krzysztof Kozlowski, Conor Dooley, Lars-Peter Clausen, Rob Herring, Uwe Kleine-König, linux-iio, devicetree, linux-kernel On Tue, Dec 26, 2023 at 10:18:47AM +0100, Krzysztof Kozlowski wrote: > On 26/12/2023 04:47, cy_huang@richtek.com wrote: > > From: ChiYuan Huang <cy_huang@richtek.com> > > > > Add compatible support for RTQ6053 and RTQ6059. > > > > Signed-off-by: ChiYuan Huang <cy_huang@richtek.com> > > --- > > .../devicetree/bindings/iio/adc/richtek,rtq6056.yaml | 5 ++++- > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > diff --git a/Documentation/devicetree/bindings/iio/adc/richtek,rtq6056.yaml b/Documentation/devicetree/bindings/iio/adc/richtek,rtq6056.yaml > > index 88e008629ea8..d1e1f36d1972 100644 > > --- a/Documentation/devicetree/bindings/iio/adc/richtek,rtq6056.yaml > > +++ b/Documentation/devicetree/bindings/iio/adc/richtek,rtq6056.yaml > > @@ -25,7 +25,10 @@ description: | > > > > properties: > > compatible: > > - const: richtek,rtq6056 > > + enum: > > + - richtek,rtq6053 > > + - richtek,rtq6056 > > Aren't these devices compatible? Your driver change says they are, so > express compatibility with list here (and oneOf). > Thanks, I try to take other binding as the reference. One more question. If rtq6053 is compatible with rtq6056, there's only chip package type difference. Do I need to seperate it into a dedicated enum element? Or just put it into one item and said this part number is compatible with rtq6056? > > Best regards, > Krzysztof > ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/2] dt-bindings: iio: adc: rtq6056: add support for the whole RTQ6056 family 2023-12-26 11:19 ` ChiYuan Huang @ 2023-12-26 12:12 ` Krzysztof Kozlowski 2023-12-28 3:19 ` ChiYuan Huang 0 siblings, 1 reply; 17+ messages in thread From: Krzysztof Kozlowski @ 2023-12-26 12:12 UTC (permalink / raw) To: ChiYuan Huang Cc: Jonathan Cameron, Krzysztof Kozlowski, Conor Dooley, Lars-Peter Clausen, Rob Herring, Uwe Kleine-König, linux-iio, devicetree, linux-kernel On 26/12/2023 12:19, ChiYuan Huang wrote: > On Tue, Dec 26, 2023 at 10:18:47AM +0100, Krzysztof Kozlowski wrote: >> On 26/12/2023 04:47, cy_huang@richtek.com wrote: >>> From: ChiYuan Huang <cy_huang@richtek.com> >>> >>> Add compatible support for RTQ6053 and RTQ6059. >>> >>> Signed-off-by: ChiYuan Huang <cy_huang@richtek.com> >>> --- >>> .../devicetree/bindings/iio/adc/richtek,rtq6056.yaml | 5 ++++- >>> 1 file changed, 4 insertions(+), 1 deletion(-) >>> >>> diff --git a/Documentation/devicetree/bindings/iio/adc/richtek,rtq6056.yaml b/Documentation/devicetree/bindings/iio/adc/richtek,rtq6056.yaml >>> index 88e008629ea8..d1e1f36d1972 100644 >>> --- a/Documentation/devicetree/bindings/iio/adc/richtek,rtq6056.yaml >>> +++ b/Documentation/devicetree/bindings/iio/adc/richtek,rtq6056.yaml >>> @@ -25,7 +25,10 @@ description: | >>> >>> properties: >>> compatible: >>> - const: richtek,rtq6056 >>> + enum: >>> + - richtek,rtq6053 >>> + - richtek,rtq6056 >> >> Aren't these devices compatible? Your driver change says they are, so >> express compatibility with list here (and oneOf). >> > Thanks, I try to take other binding as the reference. One more question. > If rtq6053 is compatible with rtq6056, there's only chip package type difference. > Do I need to seperate it into a dedicated enum element? > Or just put it into one item and said this part number is compatible with rtq6056? See example-schema. You need enum and items, both in oneOf:. Best regards, Krzysztof ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/2] dt-bindings: iio: adc: rtq6056: add support for the whole RTQ6056 family 2023-12-26 12:12 ` Krzysztof Kozlowski @ 2023-12-28 3:19 ` ChiYuan Huang 2023-12-28 7:09 ` Krzysztof Kozlowski 0 siblings, 1 reply; 17+ messages in thread From: ChiYuan Huang @ 2023-12-28 3:19 UTC (permalink / raw) To: Krzysztof Kozlowski Cc: Jonathan Cameron, Krzysztof Kozlowski, Conor Dooley, Lars-Peter Clausen, Rob Herring, Uwe Kleine-König, linux-iio, devicetree, linux-kernel On Tue, Dec 26, 2023 at 01:12:50PM +0100, Krzysztof Kozlowski wrote: > On 26/12/2023 12:19, ChiYuan Huang wrote: > > On Tue, Dec 26, 2023 at 10:18:47AM +0100, Krzysztof Kozlowski wrote: > >> On 26/12/2023 04:47, cy_huang@richtek.com wrote: > >>> From: ChiYuan Huang <cy_huang@richtek.com> > >>> > >>> Add compatible support for RTQ6053 and RTQ6059. > >>> > >>> Signed-off-by: ChiYuan Huang <cy_huang@richtek.com> > >>> --- > >>> .../devicetree/bindings/iio/adc/richtek,rtq6056.yaml | 5 ++++- > >>> 1 file changed, 4 insertions(+), 1 deletion(-) > >>> > >>> diff --git a/Documentation/devicetree/bindings/iio/adc/richtek,rtq6056.yaml b/Documentation/devicetree/bindings/iio/adc/richtek,rtq6056.yaml > >>> index 88e008629ea8..d1e1f36d1972 100644 > >>> --- a/Documentation/devicetree/bindings/iio/adc/richtek,rtq6056.yaml > >>> +++ b/Documentation/devicetree/bindings/iio/adc/richtek,rtq6056.yaml > >>> @@ -25,7 +25,10 @@ description: | > >>> > >>> properties: > >>> compatible: > >>> - const: richtek,rtq6056 > >>> + enum: > >>> + - richtek,rtq6053 > >>> + - richtek,rtq6056 > >> > >> Aren't these devices compatible? Your driver change says they are, so > >> express compatibility with list here (and oneOf). > >> > > Thanks, I try to take other binding as the reference. One more question. > > If rtq6053 is compatible with rtq6056, there's only chip package type difference. > > Do I need to seperate it into a dedicated enum element? > > Or just put it into one item and said this part number is compatible with rtq6056? > > See example-schema. You need enum and items, both in oneOf:. > After reading the 'example-schema', I Still cannot understand what the special case items means. But in my case, is the below change correct? [Diff] properties: compatible: - enum: - - richtek,rtq6053 - - richtek,rtq6056 - - richtek,rtq6059 + oneOf: + - items: + - enum: + - richtek,rtq6053 + - richtek,rtq6056 + - richtek,rtq6059 > Best regards, > Krzysztof > ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/2] dt-bindings: iio: adc: rtq6056: add support for the whole RTQ6056 family 2023-12-28 3:19 ` ChiYuan Huang @ 2023-12-28 7:09 ` Krzysztof Kozlowski 2023-12-28 7:58 ` ChiYuan Huang 0 siblings, 1 reply; 17+ messages in thread From: Krzysztof Kozlowski @ 2023-12-28 7:09 UTC (permalink / raw) To: ChiYuan Huang Cc: Jonathan Cameron, Krzysztof Kozlowski, Conor Dooley, Lars-Peter Clausen, Rob Herring, Uwe Kleine-König, linux-iio, devicetree, linux-kernel On 28/12/2023 04:19, ChiYuan Huang wrote: > On Tue, Dec 26, 2023 at 01:12:50PM +0100, Krzysztof Kozlowski wrote: >> On 26/12/2023 12:19, ChiYuan Huang wrote: >>> On Tue, Dec 26, 2023 at 10:18:47AM +0100, Krzysztof Kozlowski wrote: >>>> On 26/12/2023 04:47, cy_huang@richtek.com wrote: >>>>> From: ChiYuan Huang <cy_huang@richtek.com> >>>>> >>>>> Add compatible support for RTQ6053 and RTQ6059. >>>>> >>>>> Signed-off-by: ChiYuan Huang <cy_huang@richtek.com> >>>>> --- >>>>> .../devicetree/bindings/iio/adc/richtek,rtq6056.yaml | 5 ++++- >>>>> 1 file changed, 4 insertions(+), 1 deletion(-) >>>>> >>>>> diff --git a/Documentation/devicetree/bindings/iio/adc/richtek,rtq6056.yaml b/Documentation/devicetree/bindings/iio/adc/richtek,rtq6056.yaml >>>>> index 88e008629ea8..d1e1f36d1972 100644 >>>>> --- a/Documentation/devicetree/bindings/iio/adc/richtek,rtq6056.yaml >>>>> +++ b/Documentation/devicetree/bindings/iio/adc/richtek,rtq6056.yaml >>>>> @@ -25,7 +25,10 @@ description: | >>>>> >>>>> properties: >>>>> compatible: >>>>> - const: richtek,rtq6056 >>>>> + enum: >>>>> + - richtek,rtq6053 >>>>> + - richtek,rtq6056 >>>> >>>> Aren't these devices compatible? Your driver change says they are, so >>>> express compatibility with list here (and oneOf). >>>> >>> Thanks, I try to take other binding as the reference. One more question. >>> If rtq6053 is compatible with rtq6056, there's only chip package type difference. >>> Do I need to seperate it into a dedicated enum element? >>> Or just put it into one item and said this part number is compatible with rtq6056? >> >> See example-schema. You need enum and items, both in oneOf:. >> > After reading the 'example-schema', I Still cannot understand what the special case items > means. What is "special case items"? > > But in my case, is the below change correct? > [Diff] > properties: > compatible: > - enum: > - - richtek,rtq6053 > - - richtek,rtq6056 > - - richtek,rtq6059 > + oneOf: > + - items: > + - enum: > + - richtek,rtq6053 > + - richtek,rtq6056 > + - richtek,rtq6059 This changes nothing, you still have just one item. The example-schema has exactly that case, so why you are coding it differently? Anyway, test your DTS with the fallback, you will see that above does not work. Best regards, Krzysztof ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/2] dt-bindings: iio: adc: rtq6056: add support for the whole RTQ6056 family 2023-12-28 7:09 ` Krzysztof Kozlowski @ 2023-12-28 7:58 ` ChiYuan Huang 2023-12-28 8:06 ` Krzysztof Kozlowski 0 siblings, 1 reply; 17+ messages in thread From: ChiYuan Huang @ 2023-12-28 7:58 UTC (permalink / raw) To: Krzysztof Kozlowski Cc: Jonathan Cameron, Krzysztof Kozlowski, Conor Dooley, Lars-Peter Clausen, Rob Herring, Uwe Kleine-König, linux-iio, devicetree, linux-kernel On Thu, Dec 28, 2023 at 08:09:35AM +0100, Krzysztof Kozlowski wrote: > On 28/12/2023 04:19, ChiYuan Huang wrote: > > On Tue, Dec 26, 2023 at 01:12:50PM +0100, Krzysztof Kozlowski wrote: > >> On 26/12/2023 12:19, ChiYuan Huang wrote: > >>> On Tue, Dec 26, 2023 at 10:18:47AM +0100, Krzysztof Kozlowski wrote: > >>>> On 26/12/2023 04:47, cy_huang@richtek.com wrote: > >>>>> From: ChiYuan Huang <cy_huang@richtek.com> > >>>>> > >>>>> Add compatible support for RTQ6053 and RTQ6059. > >>>>> > >>>>> Signed-off-by: ChiYuan Huang <cy_huang@richtek.com> > >>>>> --- > >>>>> .../devicetree/bindings/iio/adc/richtek,rtq6056.yaml | 5 ++++- > >>>>> 1 file changed, 4 insertions(+), 1 deletion(-) > >>>>> > >>>>> diff --git a/Documentation/devicetree/bindings/iio/adc/richtek,rtq6056.yaml b/Documentation/devicetree/bindings/iio/adc/richtek,rtq6056.yaml > >>>>> index 88e008629ea8..d1e1f36d1972 100644 > >>>>> --- a/Documentation/devicetree/bindings/iio/adc/richtek,rtq6056.yaml > >>>>> +++ b/Documentation/devicetree/bindings/iio/adc/richtek,rtq6056.yaml > >>>>> @@ -25,7 +25,10 @@ description: | > >>>>> > >>>>> properties: > >>>>> compatible: > >>>>> - const: richtek,rtq6056 > >>>>> + enum: > >>>>> + - richtek,rtq6053 > >>>>> + - richtek,rtq6056 > >>>> > >>>> Aren't these devices compatible? Your driver change says they are, so > >>>> express compatibility with list here (and oneOf). > >>>> > >>> Thanks, I try to take other binding as the reference. One more question. > >>> If rtq6053 is compatible with rtq6056, there's only chip package type difference. > >>> Do I need to seperate it into a dedicated enum element? > >>> Or just put it into one item and said this part number is compatible with rtq6056? > >> > >> See example-schema. You need enum and items, both in oneOf:. > >> > > After reading the 'example-schema', I Still cannot understand what the special case items > > means. > > What is "special case items"? > I may misunderstand something. The special case is the 'fallback' that you mentaioned in the last. > > > > But in my case, is the below change correct? > > [Diff] > > properties: > > compatible: > > - enum: > > - - richtek,rtq6053 > > - - richtek,rtq6056 > > - - richtek,rtq6059 > > + oneOf: > > + - items: > > + - enum: > > + - richtek,rtq6053 > > + - richtek,rtq6056 > > + - richtek,rtq6059 > > This changes nothing, you still have just one item. The example-schema > has exactly that case, so why you are coding it differently? > > Anyway, test your DTS with the fallback, you will see that above does > not work. > I rewrite the below one and tested. it seems correct. [Diff] compatible: - enum: - - richtek,rtq6053 - - richtek,rtq6056 - - richtek,rtq6059 + oneOf: + - enum: + - richtek,rtq6053 + - richtek,rtq6059 + - items: + - const: richtek,rtq6056 Just one more question. If rtq6053 is fully compatibie with rtq6056, does it need to be put into oneOf enum or be just put into items enum and use the 'fallback' mechanism? If so, the 'richtek,rtq6053' in of_device_id match can be removed. Though it just remove one line, but less-changed would be better. ChiYuan. Best regards, > Krzysztof > ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/2] dt-bindings: iio: adc: rtq6056: add support for the whole RTQ6056 family 2023-12-28 7:58 ` ChiYuan Huang @ 2023-12-28 8:06 ` Krzysztof Kozlowski 2023-12-28 8:44 ` ChiYuan Huang 0 siblings, 1 reply; 17+ messages in thread From: Krzysztof Kozlowski @ 2023-12-28 8:06 UTC (permalink / raw) To: ChiYuan Huang Cc: Jonathan Cameron, Krzysztof Kozlowski, Conor Dooley, Lars-Peter Clausen, Rob Herring, Uwe Kleine-König, linux-iio, devicetree, linux-kernel On 28/12/2023 08:58, ChiYuan Huang wrote: > On Thu, Dec 28, 2023 at 08:09:35AM +0100, Krzysztof Kozlowski wrote: >> On 28/12/2023 04:19, ChiYuan Huang wrote: >>> On Tue, Dec 26, 2023 at 01:12:50PM +0100, Krzysztof Kozlowski wrote: >>>> On 26/12/2023 12:19, ChiYuan Huang wrote: >>>>> On Tue, Dec 26, 2023 at 10:18:47AM +0100, Krzysztof Kozlowski wrote: >>>>>> On 26/12/2023 04:47, cy_huang@richtek.com wrote: >>>>>>> From: ChiYuan Huang <cy_huang@richtek.com> >>>>>>> >>>>>>> Add compatible support for RTQ6053 and RTQ6059. >>>>>>> >>>>>>> Signed-off-by: ChiYuan Huang <cy_huang@richtek.com> >>>>>>> --- >>>>>>> .../devicetree/bindings/iio/adc/richtek,rtq6056.yaml | 5 ++++- >>>>>>> 1 file changed, 4 insertions(+), 1 deletion(-) >>>>>>> >>>>>>> diff --git a/Documentation/devicetree/bindings/iio/adc/richtek,rtq6056.yaml b/Documentation/devicetree/bindings/iio/adc/richtek,rtq6056.yaml >>>>>>> index 88e008629ea8..d1e1f36d1972 100644 >>>>>>> --- a/Documentation/devicetree/bindings/iio/adc/richtek,rtq6056.yaml >>>>>>> +++ b/Documentation/devicetree/bindings/iio/adc/richtek,rtq6056.yaml >>>>>>> @@ -25,7 +25,10 @@ description: | >>>>>>> >>>>>>> properties: >>>>>>> compatible: >>>>>>> - const: richtek,rtq6056 >>>>>>> + enum: >>>>>>> + - richtek,rtq6053 >>>>>>> + - richtek,rtq6056 >>>>>> >>>>>> Aren't these devices compatible? Your driver change says they are, so >>>>>> express compatibility with list here (and oneOf). >>>>>> >>>>> Thanks, I try to take other binding as the reference. One more question. >>>>> If rtq6053 is compatible with rtq6056, there's only chip package type difference. >>>>> Do I need to seperate it into a dedicated enum element? >>>>> Or just put it into one item and said this part number is compatible with rtq6056? >>>> >>>> See example-schema. You need enum and items, both in oneOf:. >>>> >>> After reading the 'example-schema', I Still cannot understand what the special case items >>> means. >> >> What is "special case items"? >> > I may misunderstand something. The special case is the 'fallback' that you mentaioned in > the last. >>> >>> But in my case, is the below change correct? >>> [Diff] >>> properties: >>> compatible: >>> - enum: >>> - - richtek,rtq6053 >>> - - richtek,rtq6056 >>> - - richtek,rtq6059 >>> + oneOf: >>> + - items: >>> + - enum: >>> + - richtek,rtq6053 >>> + - richtek,rtq6056 >>> + - richtek,rtq6059 >> >> This changes nothing, you still have just one item. The example-schema >> has exactly that case, so why you are coding it differently? >> >> Anyway, test your DTS with the fallback, you will see that above does >> not work. >> > I rewrite the below one and tested. it seems correct. > > [Diff] > compatible: > - enum: > - - richtek,rtq6053 > - - richtek,rtq6056 > - - richtek,rtq6059 > + oneOf: > + - enum: > + - richtek,rtq6053 > + - richtek,rtq6059 > + - items: > + - const: richtek,rtq6056 You still need two items here to express compatibility. What is compatible with what? It must be rtq6053 compatible with rtq6056, because you cannot break the ABI, can you? > > Just one more question. If rtq6053 is fully compatibie with rtq6056, does it need to be put > into oneOf enum or be just put into items enum and use the 'fallback' mechanism? The fallback is just a term. The point is to have a list of two compatibles. See: Devicetree specification, writing-bindings and numerous presentations about writing DTS. > > If so, the 'richtek,rtq6053' in of_device_id match can be removed. Though it just remove > one line, but less-changed would be better. The device using fallback should be removed the driver of_device_id, so 6053 goes away. Best regards, Krzysztof ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/2] dt-bindings: iio: adc: rtq6056: add support for the whole RTQ6056 family 2023-12-28 8:06 ` Krzysztof Kozlowski @ 2023-12-28 8:44 ` ChiYuan Huang 0 siblings, 0 replies; 17+ messages in thread From: ChiYuan Huang @ 2023-12-28 8:44 UTC (permalink / raw) To: Krzysztof Kozlowski Cc: Jonathan Cameron, Krzysztof Kozlowski, Conor Dooley, Lars-Peter Clausen, Rob Herring, Uwe Kleine-König, linux-iio, devicetree, linux-kernel Hi, Krzysztof: On Thu, Dec 28, 2023 at 09:06:30AM +0100, Krzysztof Kozlowski wrote: > On 28/12/2023 08:58, ChiYuan Huang wrote: > > On Thu, Dec 28, 2023 at 08:09:35AM +0100, Krzysztof Kozlowski wrote: > >> On 28/12/2023 04:19, ChiYuan Huang wrote: > >>> On Tue, Dec 26, 2023 at 01:12:50PM +0100, Krzysztof Kozlowski wrote: > >>>> On 26/12/2023 12:19, ChiYuan Huang wrote: > >>>>> On Tue, Dec 26, 2023 at 10:18:47AM +0100, Krzysztof Kozlowski wrote: > >>>>>> On 26/12/2023 04:47, cy_huang@richtek.com wrote: > >>>>>>> From: ChiYuan Huang <cy_huang@richtek.com> > >>>>>>> > >>>>>>> Add compatible support for RTQ6053 and RTQ6059. > >>>>>>> > >>>>>>> Signed-off-by: ChiYuan Huang <cy_huang@richtek.com> > >>>>>>> --- > >>>>>>> .../devicetree/bindings/iio/adc/richtek,rtq6056.yaml | 5 ++++- > >>>>>>> 1 file changed, 4 insertions(+), 1 deletion(-) > >>>>>>> > >>>>>>> diff --git a/Documentation/devicetree/bindings/iio/adc/richtek,rtq6056.yaml b/Documentation/devicetree/bindings/iio/adc/richtek,rtq6056.yaml > >>>>>>> index 88e008629ea8..d1e1f36d1972 100644 > >>>>>>> --- a/Documentation/devicetree/bindings/iio/adc/richtek,rtq6056.yaml > >>>>>>> +++ b/Documentation/devicetree/bindings/iio/adc/richtek,rtq6056.yaml > >>>>>>> @@ -25,7 +25,10 @@ description: | > >>>>>>> > >>>>>>> properties: > >>>>>>> compatible: > >>>>>>> - const: richtek,rtq6056 > >>>>>>> + enum: > >>>>>>> + - richtek,rtq6053 > >>>>>>> + - richtek,rtq6056 > >>>>>> > >>>>>> Aren't these devices compatible? Your driver change says they are, so > >>>>>> express compatibility with list here (and oneOf). > >>>>>> > >>>>> Thanks, I try to take other binding as the reference. One more question. > >>>>> If rtq6053 is compatible with rtq6056, there's only chip package type difference. > >>>>> Do I need to seperate it into a dedicated enum element? > >>>>> Or just put it into one item and said this part number is compatible with rtq6056? > >>>> > >>>> See example-schema. You need enum and items, both in oneOf:. > >>>> > >>> After reading the 'example-schema', I Still cannot understand what the special case items > >>> means. > >> > >> What is "special case items"? > >> > > I may misunderstand something. The special case is the 'fallback' that you mentaioned in > > the last. > >>> > >>> But in my case, is the below change correct? > >>> [Diff] > >>> properties: > >>> compatible: > >>> - enum: > >>> - - richtek,rtq6053 > >>> - - richtek,rtq6056 > >>> - - richtek,rtq6059 > >>> + oneOf: > >>> + - items: > >>> + - enum: > >>> + - richtek,rtq6053 > >>> + - richtek,rtq6056 > >>> + - richtek,rtq6059 > >> > >> This changes nothing, you still have just one item. The example-schema > >> has exactly that case, so why you are coding it differently? > >> > >> Anyway, test your DTS with the fallback, you will see that above does > >> not work. > >> > > I rewrite the below one and tested. it seems correct. > > > > [Diff] > > compatible: > > - enum: > > - - richtek,rtq6053 > > - - richtek,rtq6056 > > - - richtek,rtq6059 > > + oneOf: > > + - enum: > > + - richtek,rtq6053 > > + - richtek,rtq6059 > > + - items: > > + - const: richtek,rtq6056 > > You still need two items here to express compatibility. What is > compatible with what? It must be rtq6053 compatible with rtq6056, > because you cannot break the ABI, can you? > > > > > Just one more question. If rtq6053 is fully compatibie with rtq6056, does it need to be put > > into oneOf enum or be just put into items enum and use the 'fallback' mechanism? > > The fallback is just a term. The point is to have a list of two > compatibles. See: Devicetree specification, writing-bindings and > numerous presentations about writing DTS. > > > > > If so, the 'richtek,rtq6053' in of_device_id match can be removed. Though it just remove > > one line, but less-changed would be better. > > The device using fallback should be removed the driver of_device_id, so > 6053 goes away. > Thanks for all the information. If not misunderstanding, I'll remove "dt match id" for 6053. And the binding fix in v2 will be properties: compatible: - enum: - - richtek,rtq6053 - - richtek,rtq6056 - - richtek,rtq6059 + oneOf: + - enum: + - richtek,rtq6056 + - richtek,rtq6059 + - items: + - enum: + - richtek,rtq6053 + - const: richtek,rtq6056 This make the 6053 compatible with 6056 with DT fallback. > Best regards, > Krzysztof > ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/2] dt-bindings: iio: adc: rtq6056: add support for the whole RTQ6056 family 2023-12-26 9:18 ` [PATCH 1/2] dt-bindings: iio: adc: rtq6056: add support for the whole RTQ6056 family Krzysztof Kozlowski 2023-12-26 11:19 ` ChiYuan Huang @ 2023-12-26 16:24 ` Jonathan Cameron 2023-12-28 3:26 ` ChiYuan Huang 1 sibling, 1 reply; 17+ messages in thread From: Jonathan Cameron @ 2023-12-26 16:24 UTC (permalink / raw) To: Krzysztof Kozlowski Cc: cy_huang, Krzysztof Kozlowski, Conor Dooley, Lars-Peter Clausen, Rob Herring, Uwe Kleine-König, linux-iio, devicetree, linux-kernel On Tue, 26 Dec 2023 10:18:47 +0100 Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > On 26/12/2023 04:47, cy_huang@richtek.com wrote: > > From: ChiYuan Huang <cy_huang@richtek.com> > > > > Add compatible support for RTQ6053 and RTQ6059. > > > > Signed-off-by: ChiYuan Huang <cy_huang@richtek.com> > > --- > > .../devicetree/bindings/iio/adc/richtek,rtq6056.yaml | 5 ++++- > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > diff --git a/Documentation/devicetree/bindings/iio/adc/richtek,rtq6056.yaml b/Documentation/devicetree/bindings/iio/adc/richtek,rtq6056.yaml > > index 88e008629ea8..d1e1f36d1972 100644 > > --- a/Documentation/devicetree/bindings/iio/adc/richtek,rtq6056.yaml > > +++ b/Documentation/devicetree/bindings/iio/adc/richtek,rtq6056.yaml > > @@ -25,7 +25,10 @@ description: | > > > > properties: > > compatible: > > - const: richtek,rtq6056 > > + enum: > > + - richtek,rtq6053 > > + - richtek,rtq6056 > > Aren't these devices compatible? Your driver change says they are, so > express compatibility with list here (and oneOf). I'm not seeing this thread on lore.kernel.org or in my local email. Perhaps something went wrong? Jonathan > > > Best regards, > Krzysztof > ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/2] dt-bindings: iio: adc: rtq6056: add support for the whole RTQ6056 family 2023-12-26 16:24 ` Jonathan Cameron @ 2023-12-28 3:26 ` ChiYuan Huang 2023-12-28 3:37 ` ChiYuan Huang 0 siblings, 1 reply; 17+ messages in thread From: ChiYuan Huang @ 2023-12-28 3:26 UTC (permalink / raw) To: Jonathan Cameron Cc: Krzysztof Kozlowski, Krzysztof Kozlowski, Conor Dooley, Lars-Peter Clausen, Rob Herring, Uwe Kleine-König, linux-iio, devicetree, linux-kernel On Tue, Dec 26, 2023 at 04:24:50PM +0000, Jonathan Cameron wrote: > On Tue, 26 Dec 2023 10:18:47 +0100 > Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > > > On 26/12/2023 04:47, cy_huang@richtek.com wrote: > > > From: ChiYuan Huang <cy_huang@richtek.com> > > > > > > Add compatible support for RTQ6053 and RTQ6059. > > > > > > Signed-off-by: ChiYuan Huang <cy_huang@richtek.com> > > > --- > > > .../devicetree/bindings/iio/adc/richtek,rtq6056.yaml | 5 ++++- > > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > > > diff --git a/Documentation/devicetree/bindings/iio/adc/richtek,rtq6056.yaml b/Documentation/devicetree/bindings/iio/adc/richtek,rtq6056.yaml > > > index 88e008629ea8..d1e1f36d1972 100644 > > > --- a/Documentation/devicetree/bindings/iio/adc/richtek,rtq6056.yaml > > > +++ b/Documentation/devicetree/bindings/iio/adc/richtek,rtq6056.yaml > > > @@ -25,7 +25,10 @@ description: | > > > > > > properties: > > > compatible: > > > - const: richtek,rtq6056 > > > + enum: > > > + - richtek,rtq6053 > > > + - richtek,rtq6056 > > > > Aren't these devices compatible? Your driver change says they are, so > > express compatibility with list here (and oneOf). > > I'm not seeing this thread on lore.kernel.org or in my local email. > Perhaps something went wrong? > I also cannot find the original message in lore.kernel.org. But I checked the to/cc list. Nothing is lost. Still not sure what happened. Best Regards, ChiYuan > Jonathan > > > > > > > > Best regards, > > Krzysztof > > > ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/2] dt-bindings: iio: adc: rtq6056: add support for the whole RTQ6056 family 2023-12-28 3:26 ` ChiYuan Huang @ 2023-12-28 3:37 ` ChiYuan Huang 0 siblings, 0 replies; 17+ messages in thread From: ChiYuan Huang @ 2023-12-28 3:37 UTC (permalink / raw) To: Jonathan Cameron Cc: Krzysztof Kozlowski, Krzysztof Kozlowski, Conor Dooley, Lars-Peter Clausen, Rob Herring, Uwe Kleine-König, linux-iio, devicetree, linux-kernel On Thu, Dec 28, 2023 at 11:26:04AM +0800, ChiYuan Huang wrote: > On Tue, Dec 26, 2023 at 04:24:50PM +0000, Jonathan Cameron wrote: > > On Tue, 26 Dec 2023 10:18:47 +0100 > > Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > > > > > On 26/12/2023 04:47, cy_huang@richtek.com wrote: > > > > From: ChiYuan Huang <cy_huang@richtek.com> > > > > > > > > Add compatible support for RTQ6053 and RTQ6059. > > > > > > > > Signed-off-by: ChiYuan Huang <cy_huang@richtek.com> > > > > --- > > > > .../devicetree/bindings/iio/adc/richtek,rtq6056.yaml | 5 ++++- > > > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/Documentation/devicetree/bindings/iio/adc/richtek,rtq6056.yaml b/Documentation/devicetree/bindings/iio/adc/richtek,rtq6056.yaml > > > > index 88e008629ea8..d1e1f36d1972 100644 > > > > --- a/Documentation/devicetree/bindings/iio/adc/richtek,rtq6056.yaml > > > > +++ b/Documentation/devicetree/bindings/iio/adc/richtek,rtq6056.yaml > > > > @@ -25,7 +25,10 @@ description: | > > > > > > > > properties: > > > > compatible: > > > > - const: richtek,rtq6056 > > > > + enum: > > > > + - richtek,rtq6053 > > > > + - richtek,rtq6056 > > > > > > Aren't these devices compatible? Your driver change says they are, so > > > express compatibility with list here (and oneOf). > > > > I'm not seeing this thread on lore.kernel.org or in my local email. > > Perhaps something went wrong? > > > I also cannot find the original message in lore.kernel.org. > But I checked the to/cc list. Nothing is lost. > > Still not sure what happened. Ok, it seems the only difference is that I forget to add --thread in git format-patch. But not sure for that. When I send v2 patch for the binding fix, I'll check it again. Sorry for the inconvenience. > > Best Regards, > ChiYuan > > Jonathan > > > > > > > > > > > > > Best regards, > > > Krzysztof > > > > > ^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2023-12-28 8:44 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-12-28 3:51 [PATCH 0/2] RTQ6056: Add compatible for the same chip family cy_huang
2023-12-28 3:51 ` [PATCH 1/2] dt-bindings: iio: adc: rtq6056: add support for the whole RTQ6056 family cy_huang
2023-12-28 7:15 ` Krzysztof Kozlowski
2023-12-28 3:51 ` [PATCH 2/2] iio: adc: rtq6056: Add " cy_huang
2023-12-28 7:16 ` [PATCH 0/2] RTQ6056: Add compatible for the same chip family Krzysztof Kozlowski
2023-12-28 7:27 ` ChiYuan Huang
[not found] <1703562468-29052-1-git-send-email-cy_huang@richtek.com>
[not found] ` <1703562468-29052-2-git-send-email-cy_huang@richtek.com>
2023-12-26 9:18 ` [PATCH 1/2] dt-bindings: iio: adc: rtq6056: add support for the whole RTQ6056 family Krzysztof Kozlowski
2023-12-26 11:19 ` ChiYuan Huang
2023-12-26 12:12 ` Krzysztof Kozlowski
2023-12-28 3:19 ` ChiYuan Huang
2023-12-28 7:09 ` Krzysztof Kozlowski
2023-12-28 7:58 ` ChiYuan Huang
2023-12-28 8:06 ` Krzysztof Kozlowski
2023-12-28 8:44 ` ChiYuan Huang
2023-12-26 16:24 ` Jonathan Cameron
2023-12-28 3:26 ` ChiYuan Huang
2023-12-28 3:37 ` ChiYuan Huang
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).