Messages in this thread | | | From | Andy Shevchenko <> | Date | Tue, 28 Jun 2022 13:52:34 +0200 | Subject | Re: [PATCH v3 07/14] mfd: mt6370: Add Mediatek MT6370 support |
| |
On Fri, Jun 24, 2022 at 12:19 PM ChiaEn Wu <peterwu.pub@gmail.com> wrote: > Andy Shevchenko <andy.shevchenko@gmail.com> 於 2022年6月24日 週五 凌晨2:01寫道: > > On Thu, Jun 23, 2022 at 1:59 PM ChiaEn Wu <peterwu.pub@gmail.com> wrote:
...
> > > obj-$(CONFIG_INTEL_SOC_PMIC_BXTWC) += intel_soc_pmic_bxtwc.o > > > obj-$(CONFIG_INTEL_SOC_PMIC_CHTWC) += intel_soc_pmic_chtwc.o > > > obj-$(CONFIG_INTEL_SOC_PMIC_CHTDC_TI) += intel_soc_pmic_chtdc_ti.o > > > obj-$(CONFIG_MFD_MT6360) += mt6360-core.o > > > +obj-$(CONFIG_MFD_MT6370) += mt6370.o > > > mt6397-objs := mt6397-core.o mt6397-irq.o mt6358-irq.o > > > obj-$(CONFIG_MFD_MT6397) += mt6397.o > > > obj-$(CONFIG_INTEL_SOC_PMIC_MRFLD) += intel_soc_pmic_mrfld.o > > > > This whole bunch of drivers is in the wrong place in Makefile. > > > > https://lore.kernel.org/all/20220616182524.7956-2-andriy.shevchenko@linux.intel.com/ > > hmm... So shall we need to cherry-pick your this patch first, > then modify the Makefile before the next submission??
I don't know what Lee's preferences are, but at least I have these options in mind: 1) wait until Lee applies my series; 2) take that single patch to your tree as a precursor.
In the second case you will need to send the series with that patch as well.
...
> > > +#define MT6370_REG_MAXADDR 0x1FF > > > > Wondering if (BIT(10) - 1) gives a better hint on how hardware limits > > this (so it will be clear it's 10-bit address). > > well... This "0x1FF" is just a virtual mapping value to map the max > address of the PMU bank(0x1XX). > So, I feel its means is different from using (BIT(10) - 1) here.
Perhaps a comment then?
...
> > > + if (ret < 0) > > > + return ret; > > > + else if (ret != val_size) > > > > Redundant 'else'. > > I'm not quite sure what you mean, so I made the following changes first. > ------------------------------------ > if (ret < 0) > return ret; > if (ret != val_size) > return -EIO; > ------------------------------------ > I don't know if it meets your expectations??
Yes.
> > > + return -EIO;
-- With Best Regards, Andy Shevchenko
| |