Date: Wed, 19 Aug 2015 08:02:37 +0800
From: Peter Chen <peter.chen@freescale.com>
To: Mark Brown <broonie@kernel.org>
CC: Baolin Wang <baolin.wang@linaro.org>, <balbi@ti.com>,
        <linus.walleij@linaro.org>, <linux-kernel@vger.kernel.org>,
        <gregkh@linuxfoundation.org>, <sojka@merica.cz>,
        <stern@rowland.harvard.edu>, <r.baldyga@samsung.com>,
        <yoshihiro.shimoda.uh@renesas.com>, <linux-usb@vger.kernel.org>,
        <device-mainlining@lists.linuxfoundation.org>, <sre@kernel.org>,
        <dbaryshkov@gmail.com>, <dwmw2@infradead.org>, <sameo@linux.intel.com>,
        <lee.jones@linaro.org>, <patches@opensource.wolfsonmicro.com>,
        <linux-pm@vger.kernel.org>
Subject: Re: [PATCH v2 3/3] power: wm831x_power: Support USB charger current
 limit management
Message-ID: <20150819000236.GA23719@shlinux2>
References: <20150818052011.GF1864@shlinux2>
 <20150818161200.GK10748@sirena.org.uk>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <20150818161200.GK10748@sirena.org.uk>
User-Agent: Mutt/1.5.21 (2010-09-15)
SpamDiagnosticOutput: 1:23
SpamDiagnosticMetadata: NSPM
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 19 Aug 2015 01:13:45.5614
 (UTC)
X-MS-Exchange-CrossTenant-Id: 710a03f5-10f6-4d38-9ff4-a80b81da590d
X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=710a03f5-10f6-4d38-9ff4-a80b81da590d;Ip=[192.88.168.50];Helo=[tx30smr01.am.freescale.net]
X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM2PR0301MB1280
X-Microsoft-Exchange-Diagnostics: 1;DM2PR0301MB0783;2:WX+UTx1l8MatyDLuyag7uNnyGK2BuEkUxc7XBES8DnZ6VkmKAdFShNv+fQ06Mx6ozSnGX9xarWf8AFmvSuton6EJNH95E1PdZ/DCitY00GNqrnJ8CD6VPC2eBld0Z3aOX0Iw/+ypUOAt0eeHGFKzSAWVHB4SozC2iIdhvHM0IZM=;3:3g6W7thmRWG2RvSeJwvH75lgfpxTgn4ihRbiMyrBdiYt2E11geh7ZQFqONFeEEWC1tPh25gqTfBzzUAgK6CzK5d7NYd8hUuEIUUrgykECGwFvW7hrhn4qPoHScBMZSuuK9y9GEuzw/lGBssZamKbcx/nmJlxPKu5HtkIr9Od3tYZhLv5YFb2Etkb81CoeESnODYluPCYiH6vJTFyhqUjXpCcstCgfjdEdXgA9TlPGYo=;25:grAwZAGC9yvP/9iKylRFP7XieyGiOJWNfoNaFFMbb0lCtDqNHh8VSjEF00ifuthQUlV6Pk3lhaDinjECcuoBwX221tmAc4zctjCMvTZY8qUKfWPPGCzp8ALon9AFNGrgp5gfJlOwUdlXqYZB8K8ImAWLC42cq7ioMvGGRgBnxeg5Ymm45qNTlc/bw7gdymxDykmTc7DV2yX1K+NNLXZ4bIEj9YcB7sJdheqUeFYK+M4Ojl32rNPUL94goU0+kZkt;23:qkXcgIEHH94+Bw9ezVVw50jFgHKMxYU2gDh6Wv3SQdTXqZxAsvTUJnvgn9GWWuIbppSHg+uYqqZ+032WUDvv63Q0rO7OJf6VgnT4BjxdS6z7r5UscLLPsCCmsg6Umk2H/WpMlxgXY0NIyReTrwkFE+YEb/UlxJvQJHCaUleLLq4BqPecExFk8KcCfQ3Q3biL
X-OriginatorOrg: freescale.com
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
Content-Length: 2607
Lines: 77

On Tue, Aug 18, 2015 at 09:12:00AM -0700, Mark Brown wrote:
> On Tue, Aug 18, 2015 at 01:20:12PM +0800, Peter Chen wrote:
> 
> > ok, I just had suspected below function's correctness, after looking
> > it again, it always set 1800 as charging limit, does it be expected?
> 
> > +       /* Find the highest supported limit */
> > +       best = 0;
> > +       for (i = 0; i < ARRAY_SIZE(wm831x_usb_limits); i++) {
> > +               if (limit < wm831x_usb_limits[i] &&
> 

+/* In miliamps */
+static unsigned int wm831x_usb_limits[] = {
+       0,
+       2,
+       100,
+       500,
+       900,
+       1500,
+       1800,
+       550,
+};
+
+static int wm831x_usb_limit_change(struct notifier_block *nb,
+                                  unsigned long limit, void *data)
+{
+       struct wm831x_power *wm831x_power = container_of(nb,
+                                                        struct wm831x_power,
+                                                        usb_notify);
+       int i, best;
+
+       /* Find the highest supported limit */
+       best = 0;
+       for (i = 0; i < ARRAY_SIZE(wm831x_usb_limits); i++) {
+               if (limit < wm831x_usb_limits[i] &&
+                   wm831x_usb_limits[best] < wm831x_usb_limits[i])
+                       best = i;
+       }
+
+       dev_dbg(wm831x_power->wm831x->dev,
+               "Limiting USB current to %dmA", wm831x_usb_limits[best]);
+
+       wm831x_set_bits(wm831x_power->wm831x, WM831X_POWER_STATE,
+                       WM831X_USB_ILIM_MASK, best);
+
+       return 0;
+}
+

> The above check is intended to ensure that we don't go over the limit
> that was passed in in the callback.  The goal is to select the highest
> option that is less than the limit passed in.

Please correct me if I was wrong, If there is SDP (PC), the current
limit is 500mA, then when the i = 4, (500 < 900) && (0 < 900) is true,
the best is 4. When the i goes to 5, (500 < 1500) && (900 < 1500) is
true, the best is 5. At last, the best will be 1800, which is not our
expectation.

Below code may be correct for the goal you expressed.

       for (i = 0; i < ARRAY_SIZE(wm831x_usb_limits); i++) {
               if (limit >= wm831x_usb_limits[i] &&
                   wm831x_usb_limits[best] < wm831x_usb_limits[i])
                       best = i;
       }

-- 

Best Regards,
Peter Chen
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/