On Wed, 13 Jul 2022, ChiaEn Wu wrote:

Hi Lee,

Thanks for your reply.

Lee Jones [off-list ref] 於 2022年7月12日 週二 晚上11:29寫道：

quoted

On Thu, 23 Jun 2022, ChiaEn Wu wrote:

quoted

From: ChiYuan Huang <redacted>

Add Mediatek MT6370 MFD support.

No such thing as "MFD support".

And you're not getting away with submitting a 370 line patch with a 5
word change log either. :)

Please at least tell us what the device is and what it's used for.

I sincerely apologize.
We will add more descriptions of the MT6370 feature in the v5 patch.

quoted

quoted

Signed-off-by: ChiYuan Huang <redacted>
---

v3
- Refine Kconfig help text
- Refine error message of unknown vendor ID in
  mt6370_check_vendor_info()
- Refine return value handling of mt6370_regmap_read()
- Refine all probe error by using dev_err_probe()
- Refine "bank_idx" and "bank_addr" in mt6370_regmap_read() and
  mt6370_regmap_write()
- Add "#define VENID*" and drop the comments in
  mt6370_check_vendor_info()
- Drop "MFD" in MODULE_DESCRIPTION()
---
 drivers/mfd/Kconfig  |  13 ++
 drivers/mfd/Makefile |   1 +
 drivers/mfd/mt6370.c | 358 +++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 372 insertions(+)
 create mode 100644 drivers/mfd/mt6370.c

[...]

quoted

quoted

+static int mt6370_probe(struct i2c_client *i2c)
+{
+     struct mt6370_info *info;
+     struct i2c_client *usbc_i2c;
+     int ret;
+
+     info = devm_kzalloc(&i2c->dev, sizeof(*info), GFP_KERNEL);
+     if (!info)
+             return -ENOMEM;
+
+     info->dev = &i2c->dev;
+
+     usbc_i2c = devm_i2c_new_dummy_device(&i2c->dev, i2c->adapter,
+                                          MT6370_USBC_I2CADDR);
+     if (IS_ERR(usbc_i2c))
+             return dev_err_probe(&i2c->dev, PTR_ERR(usbc_i2c),
+                                  "Failed to register USBC I2C client\n");
+
+     /* Assign I2C client for PMU and TypeC */
+     info->i2c[MT6370_PMU_I2C] = i2c;
+     info->i2c[MT6370_USBC_I2C] = usbc_i2c;
+
+     info->regmap = devm_regmap_init(&i2c->dev, &mt6370_regmap_bus, info,
+                                     &mt6370_regmap_config);

Apart from in mt6370_check_vendor_info() where is this actually used?

Well... from my understanding, we use this MFD driver to make other
drivers of MT6370 (e.g. charger, ADC, led...) use the same regmap
settings.
Thus, this regmap is not only used in mt6370_check_vendor_info().

Well for that to happen you need to store the data somewhere for the
child devices to fetch from.  I don't see that happening in this
patch?  What did I miss?

-- 
Lee Jones [李琼斯]
Principal Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog