Message-ID: <20150326135412.GJ5951@x1>
Date:	Thu, 26 Mar 2015 13:54:12 +0000
From:	Lee Jones <lee.jones@...aro.org>
To:	Beomho Seo <beomho.seo@...sung.com>
Cc:	Krzysztof Kozlowski <k.kozlowski@...sung.com>,
	Sebastian Reichel <sre@...nel.org>,
	Jaewon Kim <jaewon02.kim@...sung.com>,
	linux-kernel@...r.kernel.org, devicetree@...r.kernel.org,
	linux-pm@...r.kernel.org, linux-input@...r.kernel.org,
	Inki Dae <inki.dae@...sung.com>,
	SangBae Lee <sangbae90.lee@...sung.com>,
	Rob Herring <robh+dt@...nel.org>,
	Pawel Moll <pawel.moll@....com>,
	Mark Rutland <mark.rutland@....com>,
	Ian Campbell <ijc+devicetree@...lion.org.uk>,
	Kumar Gala <galak@...eaurora.org>,
	Chanwoo Choi <cw00.choi@...sung.com>,
	Dmitry Torokhov <dmitry.torokhov@...il.com>
Subject: Re: [PATCH v7 2/5] power: max77843_charger: Add Max77843 charger
 device driver

On Thu, 26 Mar 2015, Beomho Seo wrote:
> On 03/24/2015 05:38 PM, Krzysztof Kozlowski wrote:
> > 2015-03-24 9:01 GMT+01:00 Beomho Seo <beomho.seo@...sung.com>:
> >> On 03/10/2015 10:44 PM, Beomho Seo wrote:
> >>> On 03/09/2015 09:13 PM, Krzysztof Kozlowski wrote:
> >>>> On pon, 2015-03-09 at 20:46 +0900, Beomho Seo wrote:
> >>>>> On 03/09/2015 08:02 PM, Krzysztof Kozlowski wrote:
> >>>>>> 2015-03-09 1:35 GMT+01:00 Beomho Seo <beomho.seo@...sung.com>:
> >>>>>>> On 03/08/2015 05:13 AM, Sebastian Reichel wrote:
> >>>>>>>> On Mon, Mar 02, 2015 at 07:10:35PM +0900, Jaewon Kim wrote:
> >>>>>>>>> From: Beomho Seo <beomho.seo@...sung.com>
> >>>>>>>>>
> >>>>>>>>> This patch adds device driver of max77843 charger. This driver provide
> >>>>>>>>> initialize each charging mode(e.g. fast charge, top-off mode and constant
> >>>>>>>>> charging mode so on.). Additionally, control charging paramters to use
> >>>>>>>>> i2c interface.
> >>>>>>>>>
> >>>>>>>>> Cc: Sebastian Reichel <sre@...nel.org>
> >>>>>>>>> Signed-off-by: Beomho Seo <beomho.seo@...sung.com>
> >>>>>>>>
> >>>>>>>> Reviewed-By: Sebastian Reichel <sre@...nel.org>
> >>>>>>>>
> >>>>>>>> I can't take it as is, since it depends on the private header file
> >>>>>>>> of PATCHv1.
> >>>>>>>>
> >>>>>>>> -- Sebastian
> >>>>>>>>
> >>>>>>>
> >>>>>>> This patch reviewed by Sebastian.
> >>>>>>> Could you Please merge that your git tree ?
> >>>>>>
> >>>>>> Hi,
> >>>>>>
> >>>>>> ... and again we are adding a new driver for very similar chipset to
> >>>>>> already supported. I looked at spec and the charger's registers are
> >>>>>> almost the same as for max77693. Their layout and addresses are the
> >>>>>> same. I see some minor differences, probably the most important would
> >>>>>> be different values current (fast-charge, top-off). But still 90% of
> >>>>>> registers are the same... Do we really have to add new driver?
> >>>>>>
> >>>>>> Best regards,
> >>>>>> Krzysztof
> >>>>>>
> >>>>>
> >>>>> Hi,
> >>>>>
> >>>>> Thank you for your comment. As you say, both chip set are similar.
> >>>>> But new driver need for support max77843. It is support different below
> >>>>> - Provide Battery presence information.
> >>>>
> >>>> Another set of power supply properties could be added for that chip.
> >>>> This way the get_property() function would be the same but actually the
> >>>> POWER_SUPPLY_PROP_PRESENT won't be called for max77693.
> >>>>
> >>>>> - Can OTG FET control.
> >>>>
> >>>> Where the OTG FET feature is it enabled in your driver? I couldn't find
> >>>> it.
> >>>>
> >>>
> >>> Sorry. This driver don't control OTG FET feature.
> >>>
> >>>>> - Bigger Fast charge current, Top Off current Threshold selection.
> >>>>> - Various and bigger OTG current limitation.
> >>>>> - Bigger primary charger termination voltage setting.
> >>>>> - Different maximum input current limit selection(Different step).
> >>>>
> >>>> Yes, I mentioned some of these differences (the Fast/top-off
> >>>> differences). These are differences in values so it does not require new
> >>>> driver. There is need to develop new driver just to support different
> >>>> current (3.0 A instead of 2.1 A) or voltage threshold.
> >>>>
> >>>
> >>> They are different charging current, OTG current limitation, top off current,
> >>> charging limitation value. In case OTG current limitation different not
> >>> limitation value but using register bit(max77843 use[7:6] max77693 use[7]
> >>> bit only). Even if this driver not support all feature, some register
> >>> different with max77693(support value, use register bit).
> >>>
> >>> If this driver will combined with max77693 may even be beneficial for
> >>> new Maxim driver. But the present, this driver is related with
> >>> max77843 core driver and max77843-regulator. So I hope this driver
> >>> merge first. And then will extend two driver(max77843 charger and max77693 charger).
> > 
> > I still prefer merging common drivers into one instead of creating
> > some more of them.
> > However I understand your point and I am not entirely opposed against.
> > Especially that you invested quite a bit of time for developing this
> > and my feedback was quite late. To summarize I am fine with your
> > approach.
> > 
> > Best regards,
> > Krzysztof
> >
> 
> Dear Lee Jones,
> 
> Could you please merge that your git tree ?

Sorry, I'm lost.  Why am I taking this though the MFD tree?  What
patches are left?  Where are they going?  Am I taking any other
patches?

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists