* Re: [RESEND PATCH v3 3/6] charger: max14577: Configure battery-dependent settings from DTS and sysfs
From: Lee Jones @ 2014-08-27 13:50 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Sebastian Reichel, Dmitry Eremin-Solenikov, David Woodhouse,
linux-kernel, linux-api, linux-pm, Samuel Ortiz, Liam Girdwood,
Mark Brown, Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell,
Kumar Gala, Kyungmin Park, Marek Szyprowski,
Bartlomiej Zolnierkiewicz, Jenny Tc
In-Reply-To: <1409146282-25075-4-git-send-email-k.kozlowski@samsung.com>
On Wed, 27 Aug 2014, Krzysztof Kozlowski wrote:
> Remove hard-coded values for:
> - Fast Charge current,
> - End Of Charge current,
> - Fast Charge timer,
> - Overvoltage Protection Threshold,
> - Battery Constant Voltage,
> and use DTS or sysfs to configure them. This allows using the max14577 charger
> driver with different batteries.
>
> Now the charger driver requires valid configuration data from DTS. In
> case of wrong configuration data it fails during probe. Patch adds
> of_compatible to the charger mfd cell in MFD driver core.
>
> The fast charge timer is configured through sysfs entry.
>
> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> Cc: Kyungmin Park <kyungmin.park@samsung.com>
> Cc: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
> Cc: David Woodhouse <dwmw2@infradead.org>
> Cc: Jenny Tc <jenny.tc@intel.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Acked-by: Lee Jones <lee.jones@linaro.org>
> ---
> drivers/mfd/max14577.c | 5 +-
Hmm... not sure why I Acked this. The code looks okay, but can you
break out the MFD changed into a separate patch please? You can add
my Ack to the broken out patch though.
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply
* Re: [RESEND PATCH v3 1/6] charger: max14577: Add support for MAX77836 charger
From: Lee Jones @ 2014-08-27 13:52 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Sebastian Reichel, Dmitry Eremin-Solenikov, David Woodhouse,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA, linux-pm-u79uwXL29TY76Z2rM5mHXA,
Samuel Ortiz, Liam Girdwood, Mark Brown, Rob Herring, Pawel Moll,
Mark Rutland, Ian Campbell, Kumar Gala, Kyungmin Park,
Marek Szyprowski, Bartlomiej Zolnierkiewicz, Anton Vorontsov
In-Reply-To: <1409146282-25075-2-git-send-email-k.kozlowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
On Wed, 27 Aug 2014, Krzysztof Kozlowski wrote:
> Add support for MAX77836 charger to the max14577 driver. The MAX77836
> charger is almost the same as 14577 model except:
> - No dead-battery detection;
> - Support for special charger (like in MAX77693);
> - Support for DX over-voltage protection (like in MAX77693);
> - Lower values of charging current (two times lower current for
> slow/fast charge, much lower EOC current);
> - Slightly different values in ChgTyp field of STATUS2 register. On
> MAX14577 0x6 is reserved and 0x7 dead battery. On the MAX77836 the
> 0x6 means special charger and 0x7 is reserved. Regardless of these
> differences the driver maps them to one enum max14577_muic_charger_type.
>
> Signed-off-by: Krzysztof Kozlowski <k.kozlowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> Cc: Kyungmin Park <kyungmin.park-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> Cc: Anton Vorontsov <anton-9xeibp6oKSgdnm+yROfE0A@public.gmane.org>
> Cc: Dmitry Eremin-Solenikov <dbaryshkov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Cc: David Woodhouse <dwmw2-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>
> Acked-by: Lee Jones <lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
> drivers/power/Kconfig | 4 +-
> drivers/power/max14577_charger.c | 77 +++++++++++++++++++++++++++++-------
> include/linux/mfd/max14577-private.h | 54 ++++++++++++++++++-------
Not quite so important here, but better to break these out too. We
can arrange ourselves to that the set can go in together. Just make
sure it's bisectable.
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply
* Re: [RESEND PATCH v3 3/6] charger: max14577: Configure battery-dependent settings from DTS and sysfs
From: Krzysztof Kozlowski @ 2014-08-27 13:55 UTC (permalink / raw)
To: Lee Jones
Cc: Sebastian Reichel, Dmitry Eremin-Solenikov, David Woodhouse,
linux-kernel, linux-api, linux-pm, Samuel Ortiz, Liam Girdwood,
Mark Brown, Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell,
Kumar Gala, Kyungmin Park, Marek Szyprowski,
Bartlomiej Zolnierkiewicz, Jenny Tc
In-Reply-To: <20140827135027.GF6364@lee--X1>
On śro, 2014-08-27 at 14:50 +0100, Lee Jones wrote:
> On Wed, 27 Aug 2014, Krzysztof Kozlowski wrote:
>
> > Remove hard-coded values for:
> > - Fast Charge current,
> > - End Of Charge current,
> > - Fast Charge timer,
> > - Overvoltage Protection Threshold,
> > - Battery Constant Voltage,
> > and use DTS or sysfs to configure them. This allows using the max14577 charger
> > driver with different batteries.
> >
> > Now the charger driver requires valid configuration data from DTS. In
> > case of wrong configuration data it fails during probe. Patch adds
> > of_compatible to the charger mfd cell in MFD driver core.
> >
> > The fast charge timer is configured through sysfs entry.
> >
> > Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> > Cc: Kyungmin Park <kyungmin.park@samsung.com>
> > Cc: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
> > Cc: David Woodhouse <dwmw2@infradead.org>
> > Cc: Jenny Tc <jenny.tc@intel.com>
> > Cc: Mark Rutland <mark.rutland@arm.com>
> > Acked-by: Lee Jones <lee.jones@linaro.org>
> > ---
> > drivers/mfd/max14577.c | 5 +-
>
> Hmm... not sure why I Acked this. The code looks okay, but can you
> break out the MFD changed into a separate patch please? You can add
> my Ack to the broken out patch though.
Sure, I'll split it however the compatible here in mfd_cell makes sense
only with this patch.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [RESEND PATCH v3 1/6] charger: max14577: Add support for MAX77836 charger
From: Krzysztof Kozlowski @ 2014-08-27 14:01 UTC (permalink / raw)
To: Lee Jones
Cc: Sebastian Reichel, Dmitry Eremin-Solenikov, David Woodhouse,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA, linux-pm-u79uwXL29TY76Z2rM5mHXA,
Samuel Ortiz, Liam Girdwood, Mark Brown, Rob Herring, Pawel Moll,
Mark Rutland, Ian Campbell, Kumar Gala, Kyungmin Park,
Marek Szyprowski, Bartlomiej Zolnierkiewicz, Anton Vorontsov
In-Reply-To: <20140827135218.GG6364@lee--X1>
On śro, 2014-08-27 at 14:52 +0100, Lee Jones wrote:
> On Wed, 27 Aug 2014, Krzysztof Kozlowski wrote:
>
> > Add support for MAX77836 charger to the max14577 driver. The MAX77836
> > charger is almost the same as 14577 model except:
> > - No dead-battery detection;
> > - Support for special charger (like in MAX77693);
> > - Support for DX over-voltage protection (like in MAX77693);
> > - Lower values of charging current (two times lower current for
> > slow/fast charge, much lower EOC current);
> > - Slightly different values in ChgTyp field of STATUS2 register. On
> > MAX14577 0x6 is reserved and 0x7 dead battery. On the MAX77836 the
> > 0x6 means special charger and 0x7 is reserved. Regardless of these
> > differences the driver maps them to one enum max14577_muic_charger_type.
> >
> > Signed-off-by: Krzysztof Kozlowski <k.kozlowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> > Cc: Kyungmin Park <kyungmin.park-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> > Cc: Anton Vorontsov <anton-9xeibp6oKSgdnm+yROfE0A@public.gmane.org>
> > Cc: Dmitry Eremin-Solenikov <dbaryshkov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> > Cc: David Woodhouse <dwmw2-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>
> > Acked-by: Lee Jones <lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> > ---
> > drivers/power/Kconfig | 4 +-
> > drivers/power/max14577_charger.c | 77 +++++++++++++++++++++++++++++-------
> > include/linux/mfd/max14577-private.h | 54 ++++++++++++++++++-------
>
> Not quite so important here, but better to break these out too. We
> can arrange ourselves to that the set can go in together. Just make
> sure it's bisectable.
Are you sure? This will result in a patch which adds some
code/symbols/defines used only by next (future) patch. Independent apply
of such patch still won't make any sense because there won't be any user
of this code.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH 1/1] add selftest for virtio-net
From: Michael S. Tsirkin @ 2014-08-27 14:28 UTC (permalink / raw)
To: Jason Wang
Cc: Hengjinxiao, famz, netdev, linux-kernel, virtualization,
linux-api
In-Reply-To: <53FD6542.10708@redhat.com>
On Wed, Aug 27, 2014 at 12:57:38PM +0800, Jason Wang wrote:
> On 08/27/2014 09:45 AM, Hengjinxiao wrote:
> > Selftest is an important part of network driver, this patch adds selftest for
> > virtio-net, including loopback test, negotiate test and reset test. Loopback
> > test checks whether virtio-net can send and receive packets normally. Negotiate test
> > executes feature negotiation between virtio-net driver in Guest OS and virtio-net
> > device in Host OS. Reset test resets virtio-net.
>
> Thanks for the patch. Feature negotiation part brings some complicity
> and need more through. And this could be extended for CVE regression in
> the future. And you probably also need to send a patch of virtio spec to
> implement the loop back mode.
>
> See comments inline.
> >
> > Signed-off-by: Hengjinxiao <hjxiaohust@gmail.com>
> >
> > ---
> > drivers/net/virtio_net.c | 233 +++++++++++++++++++++++++++++++++++++++-
> > include/uapi/linux/virtio_net.h | 9 ++
> > 2 files changed, 241 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > index 59caa06..f83f6e4 100644
> > --- a/drivers/net/virtio_net.c
> > +++ b/drivers/net/virtio_net.c
> > @@ -28,6 +28,7 @@
> > #include <linux/cpu.h>
> > #include <linux/average.h>
> > #include <net/busy_poll.h>
> > +#include <linux/pci.h>
> >
> > static int napi_weight = NAPI_POLL_WEIGHT;
> > module_param(napi_weight, int, 0444);
> > @@ -51,6 +52,23 @@ module_param(gso, bool, 0444);
> > #define MERGEABLE_BUFFER_ALIGN max(L1_CACHE_BYTES, 256)
> >
> > #define VIRTNET_DRIVER_VERSION "1.0.0"
> > +#define __VIRTNET_TESTING 0
> > +
>
> Why need this marco?
> > +enum {
> > + VIRTNET_LOOPBACK_TEST,
> > + VIRTNET_FEATURE_NEG_TEST,
> > + VIRTNET_RESET_TEST,
> > +};
> > +
> > +static const struct {
> > + const char string[ETH_GSTRING_LEN];
> > +} virtnet_gstrings_test[] = {
> > + [VIRTNET_LOOPBACK_TEST] = { "loopback test (offline)" },
> > + [VIRTNET_FEATURE_NEG_TEST] = { "negotiate test (offline)" },
> > + [VIRTNET_RESET_TEST] = { "reset test (offline)" },
> > +};
> > +
> > +#define VIRTNET_NUM_TEST ARRAY_SIZE(virtnet_gstrings_test)
> >
> > struct virtnet_stats {
> > struct u64_stats_sync tx_syncp;
> > @@ -104,6 +122,8 @@ struct virtnet_info {
> > struct send_queue *sq;
> > struct receive_queue *rq;
> > unsigned int status;
> > + unsigned long flags;
> > + atomic_t lb_count;
> >
> > /* Max # of queue pairs supported by the device */
> > u16 max_queue_pairs;
> > @@ -436,6 +456,19 @@ err_buf:
> > return NULL;
> > }
> >
> > +void virtnet_check_lb_frame(struct virtnet_info *vi,
> > + struct sk_buff *skb)
> > +{
> > + unsigned int frame_size = skb->len;
> > +
> > + if (*(skb->data + 3) == 0xFF) {
> > + if ((*(skb->data + frame_size / 2 + 10) == 0xBE) &&
> > + (*(skb->data + frame_size / 2 + 12) == 0xAF)) {
> > + atomic_dec(&vi->lb_count);
> > + }
> > + }
> > +}
> > +
> > static void receive_buf(struct receive_queue *rq, void *buf, unsigned int len)
> > {
> > struct virtnet_info *vi = rq->vq->vdev->priv;
> > @@ -485,7 +518,12 @@ static void receive_buf(struct receive_queue *rq, void *buf, unsigned int len)
> > } else if (hdr->hdr.flags & VIRTIO_NET_HDR_F_DATA_VALID) {
> > skb->ip_summed = CHECKSUM_UNNECESSARY;
> > }
> > -
> > + /* loopback self test for ethtool */
> > + if (test_bit(__VIRTNET_TESTING, &vi->flags)) {
> > + virtnet_check_lb_frame(vi, skb);
> > + dev_kfree_skb_any(skb);
> > + return;
> > + }
>
> Not sure it's a good choice for adding such in fast path. We may need a
> test specific rx interrupt handler (and disable NAPI) for this.
I agree here.
Since it's an offline test, there's no need to mix
it up with the main mode.
> > skb->protocol = eth_type_trans(skb, dev);
> > pr_debug("Receiving skb proto 0x%04x len %i type %i\n",
> > ntohs(skb->protocol), skb->len, skb->pkt_type);
> > @@ -813,6 +851,9 @@ static int virtnet_open(struct net_device *dev)
> > {
> > struct virtnet_info *vi = netdev_priv(dev);
> > int i;
> > + /* disallow open during test */
> > + if (test_bit(__VIRTNET_TESTING, &vi->flags))
> > + return -EBUSY;
> >
> > for (i = 0; i < vi->max_queue_pairs; i++) {
> > if (i < vi->curr_queue_pairs)
> > @@ -1363,12 +1404,158 @@ static void virtnet_get_channels(struct net_device *dev,
> > channels->other_count = 0;
> > }
> >
> > +static int virtnet_reset(struct virtnet_info *vi);
> > +
Pls avoid forward declarations.
Also this seems to duplicate a bunch of code.
Please don't duplicate code, use functions to
reuse it.
> > +static void virtnet_create_lb_frame(struct sk_buff *skb,
> > + unsigned int frame_size)
> > +{
> > + memset(skb->data, 0xFF, frame_size);
> > + frame_size &= ~1;
> > + memset(&skb->data[frame_size / 2], 0xAA, frame_size / 2 - 1);
> > + memset(&skb->data[frame_size / 2 + 10], 0xBE, 1);
> > + memset(&skb->data[frame_size / 2 + 12], 0xAF, 1);
> > +}
> > +
> > +static int virtnet_start_loopback(struct virtnet_info *vi)
> > +{
> > + if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_LOOPBACK,
> > + VIRTIO_NET_CTRL_LOOPBACK_SET, NULL, NULL)) {
> > + dev_warn(&vi->dev->dev, "Failed to set loopback.\n");
> > + return -EINVAL;
> > + }
> > + return 0;
>
> You may need to test the feature bit of loop back first and report the
> card does not support loop back in some way.
> > +}
> > +
> > +static int virtnet_run_loopback_test(struct virtnet_info *vi)
> > +{
> > + int i;
> > + netdev_tx_t rc;
> > + struct sk_buff *skb;
> > + unsigned int size = GOOD_COPY_LEN;
> > +
> > + for (i = 0; i < 100; i++) {
> > + skb = netdev_alloc_skb(vi->dev, size);
> > + if (!skb)
> > + return -ENOMEM;
> > +
> > + skb->queue_mapping = 0;
> > + skb_put(skb, size);
> > + virtnet_create_lb_frame(skb, size);
> > + rc = start_xmit(skb, vi->dev);
>
> virtio_net does not use tx interrupt to free old xmit skbs. It poll tx
> completion only during xmit_skb(). So at least the last skb is leaked
> since it was not freed. A possible solution is using tx interrupt here.
> > + if (rc != NETDEV_TX_OK)
> > + return -EPIPE;
>
> It looks to me that start_xmit() never return other value than NETDEV_TX_OK.
> > + atomic_inc(&vi->lb_count);
> > + }
> > + /* Give queue time to settle before testing results. */
> > + msleep(20);
>
> Need to make sure this value is also ok for qemu. ixgbe use 200 to 64
> packets.
Yea this looks very ugly.
Do we really need hard-coded timeouts?
> > + return atomic_read(&vi->lb_count) ? -EIO : 0;
> > +}
> > +
> > +static int virtnet_stop_loopback(struct virtnet_info *vi)
> > +{
> > + if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_LOOPBACK,
> > + VIRTIO_NET_CTRL_LOOPBACK_UNSET, NULL, NULL)) {
> > + dev_warn(&vi->dev->dev, "Failed to unset loopback.\n");
> > + return -EINVAL;
> > + }
> > + return 0;
> > +}
> > +
> > +static int virtnet_loopback_test(struct virtnet_info *vi, u64 *data)
> > +{
> > + *data = virtnet_start_loopback(vi);
> > + if (*data)
> > + goto out;
> > + *data = virtnet_run_loopback_test(vi);
> > + if (*data)
>
> Do we need to stop loopback here?
> > + goto out;
> > + *data = virtnet_stop_loopback(vi);
> > +out:
> > + return *data;
> > +}
> > +
> > +static void virtnet_feature_neg_test(struct virtnet_info *vi)
> > +{
> > + struct virtio_device *dev = vi->vdev;
> > + struct virtio_driver *drv = drv_to_virtio(dev->dev.driver);
> > + int i;
> > + u32 device_features;
> > +
> > + /* Figure out what features the device supports. */
> > + device_features = dev->config->get_features(dev);
> > +
> > + /* Features supported by both device and driver into dev->features. */
> > + memset(dev->features, 0, sizeof(dev->features));
> > + for (i = 0; i < drv->feature_table_size; i++) {
> > + unsigned int f = drv->feature_table[i];
> > +
> > + BUG_ON(f >= 32);
> > + if (device_features & (1 << f))
> > + set_bit(f, dev->features);
> > + }
> > +
> > + /* Transport features always preserved to pass to finalize_features. */
> > + for (i = VIRTIO_TRANSPORT_F_START; i < VIRTIO_TRANSPORT_F_END; i++)
> > + if (device_features & (1 << i))
> > + set_bit(i, dev->features);
> > +
> > + dev->config->finalize_features(dev);
> > +}
>
> A problem of the function is it may be called during DRIVER_OK, not sure
> this is ok since spec suggest to do the feature negotiation after DRIVER
> bit but before DRIVER_OK bit. And this function duplicates some of the
> code from virtio core, may consider a method to share between them.
>
> Another issue is the test never fail which needs more thought.
> > +
> > +static int virtnet_get_sset_count(struct net_device *netdev, int sset)
> > +{
> > + switch (sset) {
> > + case ETH_SS_TEST:
> > + return VIRTNET_NUM_TEST;
> > + default:
> > + return -EOPNOTSUPP;
> > + }
> > +}
> > +
> > +static void virtnet_get_strings(struct net_device *dev, u32 stringset, u8 *buf)
> > +{
> > + switch (stringset) {
> > + case ETH_SS_TEST:
> > + memcpy(buf, &virtnet_gstrings_test,
> > + sizeof(virtnet_gstrings_test));
> > + break;
> > + default:
> > + break;
> > + }
> > +}
> > +
> > +static void virtnet_self_test(struct net_device *netdev,
> > + struct ethtool_test *eth_test, u64 *data)
> > +{
> > + struct virtnet_info *vi = netdev_priv(netdev);
> > + bool if_running = netif_running(netdev);
> > +
> > + set_bit(__VIRTNET_TESTING, &vi->flags);
> > + memset(data, 0, sizeof(u64) * VIRTNET_NUM_TEST);
> > +
> > + if (eth_test->flags == ETH_TEST_FL_OFFLINE) {
> > + if (!if_running) {
> > + dev_warn(&vi->dev->dev, "Failed to execute self test.\n");
> > + eth_test->flags |= ETH_TEST_FL_FAILED;
> > + return;
> > + }
> > + if (virtnet_loopback_test(vi, &data[VIRTNET_LOOPBACK_TEST]))
> > + eth_test->flags |= ETH_TEST_FL_FAILED;
> > + virtnet_feature_neg_test(vi);
> > + virtnet_reset(vi);
> > + }
> > + clear_bit(__VIRTNET_TESTING, &vi->flags);
> > +}
> > +
> > static const struct ethtool_ops virtnet_ethtool_ops = {
> > .get_drvinfo = virtnet_get_drvinfo,
> > .get_link = ethtool_op_get_link,
> > .get_ringparam = virtnet_get_ringparam,
> > .set_channels = virtnet_set_channels,
> > .get_channels = virtnet_get_channels,
> > + .self_test = virtnet_self_test,
> > + .get_strings = virtnet_get_strings,
> > + .get_sset_count = virtnet_get_sset_count,
> > };
> >
> > #define MIN_MTU 68
> > @@ -1957,6 +2144,50 @@ static int virtnet_restore(struct virtio_device *vdev)
> > }
> > #endif
> >
> > +static int virtnet_reset(struct virtnet_info *vi)
>
> If this is needed, better split this into anther patch.
> > +{
> > + struct virtio_device *vdev = vi->vdev;
> > + int err, i;
> > + u8 status;
> > +
> > + mutex_lock(&vi->config_lock);
> > + vi->config_enable = false;
> > + mutex_unlock(&vi->config_lock);
> > +
> > + cancel_delayed_work_sync(&vi->refill);
> > +
> > + if (netif_running(vi->dev))
> > + for (i = 0; i < vi->max_queue_pairs; i++) {
> > + napi_disable(&vi->rq[i].napi);
> > + netif_napi_del(&vi->rq[i].napi);
> > + }
> > +
> > + remove_vq_common(vi);
> > + flush_work(&vi->config_work);
> > +
> > + virtnet_feature_neg_test(vi);
>
> Is this used for feature negotiation? If yes, need a better name and can
> we reuse virtio core function to do this?
> > + err = init_vqs(vi);
> > + if (err)
> > + return err;
> > + if (netif_running(vi->dev)) {
> > + for (i = 0; i < vi->curr_queue_pairs; i++)
> > + if (!try_fill_recv(&vi->rq[i], GFP_KERNEL))
> > + schedule_delayed_work(&vi->refill, 0);
> > +
> > + for (i = 0; i < vi->max_queue_pairs; i++)
> > + virtnet_napi_enable(&vi->rq[i]);
> > + }
> > +
> > + mutex_lock(&vi->config_lock);
> > + vi->config_enable = true;
> > + mutex_unlock(&vi->config_lock);
> > +
> > + virtnet_set_queues(vi, vi->curr_queue_pairs);
> > + status = vdev->config->get_status(vdev);
> > + vdev->config->set_status(vdev, status | VIRTIO_CONFIG_S_DRIVER_OK);
> > + return 0;
> > +}
> > +
> > static struct virtio_device_id id_table[] = {
> > { VIRTIO_ID_NET, VIRTIO_DEV_ANY_ID },
> > { 0 },
> > diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h
> > index 172a7f0..1f31f90 100644
> > --- a/include/uapi/linux/virtio_net.h
> > +++ b/include/uapi/linux/virtio_net.h
> > @@ -201,4 +201,13 @@ struct virtio_net_ctrl_mq {
> > #define VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MIN 1
> > #define VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MAX 0x8000
> >
> > + /*
> > + * Control Loopback(5 is used by VIRTIO_NET_CTRL_GUEST_OFFLOADS in latest qemu)
> > + *
> > + * The command VIRTIO_NET_CTRL_LOOPBACK_SET is used to require the device come
> > + * into loopback state.
> > + */
> > +#define VIRTIO_NET_CTRL_LOOPBACK 6
> > + #define VIRTIO_NET_CTRL_LOOPBACK_SET 0
> > + #define VIRTIO_NET_CTRL_LOOPBACK_UNSET 1
> > #endif /* _LINUX_VIRTIO_NET_H */
^ permalink raw reply
* Re: [PATCH V4 3/8] namespaces: expose ns instance serial numbers in proc
From: Richard Guy Briggs @ 2014-08-27 15:17 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Linux API, Linux Containers, linux-kernel@vger.kernel.org,
linux-audit, Eric W. Biederman, netdev, Serge E. Hallyn
In-Reply-To: <CALCETrVVQ2Ugesq8zWK_gmj46Zr6mFL2Riyf=Lrdqy8ze3YfPA@mail.gmail.com>
On 14/08/25, Andy Lutomirski wrote:
> On Mon, Aug 25, 2014 at 9:41 AM, Nicolas Dichtel
> <nicolas.dichtel@6wind.com> wrote:
> > Le 25/08/2014 18:13, Andy Lutomirski a écrit :
> >
> >> On Mon, Aug 25, 2014 at 8:43 AM, Nicolas Dichtel
> >> <nicolas.dichtel@6wind.com> wrote:
> >>>
> >>> Le 25/08/2014 16:04, Andy Lutomirski a écrit :
> >>>
> >>>> On Aug 25, 2014 6:30 AM, "Nicolas Dichtel" <nicolas.dichtel@6wind.com>
> >>>> wrote:
> >>>>>>
> >>>>>>
> >>>>>> CRIU wants to save the complete state of a namespace and then restore
> >>>>>> it. For that to work, any information exposed to things in the
> >>>>>> namespace *cannot* be globally unique or unique per boot, since CRIU
> >>>>>> needs to arrange for that information to match whatever it was when
> >>>>>> CRIU saved it.
> >>>>>
> >>>>>
> >>>>>
> >>>>> How are ifindex of network devices managed? These ifindexes are unique
> >>>>> per boot,
> >>>>> thus can change depending on the order in which netdev are created.
> >>>>> These ifindexes are unique per boot and exposed to userspace ...
> >>>>>
> >>>>
> >>>> This does not appear to be true.
> >>>>
> >>>> $ sudo unshare --net
> >>>> # ip link add veth0 type veth peer name veth1
> >>>> # ip link
> >>>> 1: lo: <LOOPBACK> mtu 65536 qdisc noop state DOWN mode DEFAULT group
> >>>> default
> >>>> link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
> >>>> 2: veth1: <BROADCAST,MULTICAST> mtu 1500 qdisc noop state DOWN mode
> >>>> DEFAULT group default qlen 1000
> >>>> link/ether 06:0d:59:c7:a6:a8 brd ff:ff:ff:ff:ff:ff
> >>>> 3: veth0: <BROADCAST,MULTICAST> mtu 1500 qdisc noop state DOWN mode
> >>>> DEFAULT group default qlen 1000
> >>>> link/ether b2:5c:8b:f2:12:28 brd ff:ff:ff:ff:ff:ff
> >>>> # logout
> >>>> $ ip link
> >>>> 1: lo: <LOOPBACK,UP,LOWER_UP> mtu 65536 qdisc noqueue state UNKNOWN
> >>>> link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
> >>>> 3: em1: <NO-CARRIER,BROADCAST,MULTICAST,UP> mtu 1500 qdisc pfifo_fast
> >>>> state DOWN qlen 1000
> >>>>
> >>> I've probably misunderstood what you're trying to say. ifindexes are
> >>> unique
> >>> per
> >>> boot and per netns.
> >>
> >>
> >> I think we both misunderstood each other. The ifindexes are unique
> >> *per netns*, which means that, if you're unprivileged in a netns,
> >> global information doesn't leak to you. I think this is good.
> >
> > Ok, I agree. I think audit daemons are always running under privileged
> > users.
> >
> >
> >>
> >>>>
> >>>> Let me try again, with emphasis in the right place.
> >>>>
> >>>> I think that *code running in a namespace* has no business even
> >>>> knowing a unique identity of *that namespace* from the perspective of
> >>>> the host.
> >>>>
> >>>> In your example, if there's a veth device between netns A and netns B,
> >>>> then code *in netns A* has no business knowing the identity of its
> >>>> veth peer if its peer (B) is a sibling or ancestor. It also IMO has
> >>>> no business knowing the identity of its own netns (A) other than as
> >>>> "my netns".
> >>>
> >>>
> >>> I do not agree (see the example below).
> >>>
> >>>
> >>>>
> >>>> If A and B are siblings, then their parent needs to know where that
> >>>> veth device goes, but I think this is already the case to a sufficient
> >>>> extent today.
> >>>
> >>>
> >>> I'm not aware of a hierarchy between netns. A daemon should be able to
> >>> got the full network configuration, even if it's started when this
> >>> configuration
> >>> is already applied, ie even if it doesn't know what happen before it
> >>> starts.
> >>>
> >>
> >> I don't know exactly which namespaces have an explicit hierarchy, but
> >> there is certainly a hierarchy of *user* namespaces, and network
> >> namespaces live in user namespaces, so they at least have somewhat of
> >> a hierarchy.
> >>
> >>>
> >>>>
> >>>> I feel like this discussion is falling into a common trap of new API
> >>>> discussions. Can one of you who wants this API please articulate,
> >>>> with a reasonably precise example, what it is that you want to do, why
> >>>> you can't easily do it already, and how this API helps? I currently
> >>>> understand how the API creates problems, but I don't understand how it
> >>>> solves any problems, and I will NAK it (and I suspect that Eric will,
> >>>> too, which is pretty much fatal) unless that changes.
> >>>
> >>>
> >>> What I'm trying to solve is to have full info in netlink messages sent by
> >>> the
> >>> kernel, thus beeing able to identify a peer netns (and this is close from
> >>> what
> >>> audit guys are trying to have). Theorically, messages sent by the kernel
> >>> can
> >>> be
> >>> reused as is to have the same configuration. This is not the case with
> >>> x-netns
> >>> devices. Here is an example, with ip tunnels:
> >>>
> >>> $ ip netns add 1
> >>> $ ip link add ipip1 type ipip remote 10.16.0.121 local 10.16.0.249 dev
> >>> eth0
> >>> $ ip -d link ls ipip1
> >>> 8: ipip1@eth0: <POINTOPOINT,NOARP> mtu 1480 qdisc noop state DOWN mode
> >>> DEFAULT group default
> >>> link/ipip 10.16.0.249 peer 10.16.0.121 promiscuity 0
> >>> ipip remote 10.16.0.121 local 10.16.0.249 dev eth0 ttl inherit
> >>> pmtudisc
> >>> $ ip link set ipip1 netns 1
> >>> $ ip netns exec 1 ip -d link ls ipip1
> >>> 8: ipip1@tunl0: <POINTOPOINT,NOARP,M-DOWN> mtu 1480 qdisc noop state DOWN
> >>> mode DEFAULT group default
> >>> link/ipip 10.16.0.249 peer 10.16.0.121 promiscuity 0
> >>> ipip remote 10.16.0.121 local 10.16.0.249 dev tunl0 ttl inherit
> >>> pmtudisc
> >>>
> >>> Now informations got with 'ip link' are wrong and incomplete:
> >>> - the link dev is now tunl0 instead of eth0, because we only got an
> >>> ifindex
> >>> from the kernel without any netns informations.
> >>> - the encapsulation addresses are not part of this netns but the user
> >>> doesn't
> >>> known that (still because netns info is missing). These IPv4
> >>> addresses
> >>> may
> >>> exist into this netns.
> >>> - it's not possible to create the same netdevice with these infos.
> >>>
> >>
> >> Aha. That's a genuine problem.
> >>
> >> Perhaps we need a concept of which netnses should be able to see each
> >> other.
> >
> > Yes, I agree. This is not required for all netns, only a subset of netns
> > should
> >
> > be able to see each other.
> >
> >>
> >> I think I would be okay with a somewhat different outcome from your
> >> example:
> >>
> >> $ ip netns exec 1 ip -d link ls ipip1
> >> 8: ipip1@[unknown device in another namespace]:
> >> <POINTOPOINT,NOARP,M-DOWN> mtu 1480 qdisc noop state DOWN
> >>
> >> I think this outcome is mandatory if netns 1 lives in a subsidiary
> >> user namespace.
> >
> > Yes.
> >
> >
> >>
> >> Certainly, if you do the 'ip link' in the original namespace, I agree
> >> that this should work.
> >
> > And yes :)
> >
> > I will update my previous proposal
> > (http://thread.gmane.org/gmane.linux.network/315933/focus=321753)
> > to allow to get an id for a peer netns only when the user namespace is the
> > same.
>
> I think it should work if the peer userns is the same or a descendent.
> I also wonder whether the peer's ifindex should be suppressed if peer
> userns is not the same or a descendent.
>
> Now you just have to get Eric to be happy with the id allocation. :)
> This may be nontrivial.
>
> >> For most namespace types, this all works transparently, since
> >> everything has an real identity all the way up the hierarchy. Network
> >> namespaces are different.
> >>
> >> I don't think that exposing serial numbers in /proc is a good
> >> solution, both for the reasons already described and because I don't
> >> think that iproute2 should need to muck around with /proc to function
> >
> > A netlink API is probably enough. But it will help only for the network
> > problem, not for audit. I was hoping to find a common solution.
>
> I still don't understand why audit needs anything beyond the audit
> part of this patch set. I have no problem with audit seeing that
> migrated/restored namespaces are really brand-new namespaces, as long
> as the code in those namespaces isn't exposed to it.
Ok, I'm starting to get this... Perhaps /proc wasn't the best place to
expose this. Audit or an audit aggregator is the only one that needs to
know any of this information. This could be accomplished with
CAP_AUDIT_CONTROL and a new netlink audit message type to fetch
individual or all namespace IDs for a particular PID via auditctl, or by
having a CAP_AUDIT_WRITE-capable application pull the trigger to simply
dump that information to the log.
> >> correctly. Eric, any clever ideas here? Do we need fancier netlink
> >> messages for this?
> >>
> >> --Andy
>
> Andy Lutomirski
- RGB
--
Richard Guy Briggs <rbriggs@redhat.com>
Senior Software Engineer, Kernel Security, AMER ENG Base Operating Systems, Red Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635, Alt: +1.613.693.0684x3545
^ permalink raw reply
* [PATCH v4 0/8] charger/mfd: max14577: Add support for MAX77836
From: Krzysztof Kozlowski @ 2014-08-27 15:20 UTC (permalink / raw)
To: Sebastian Reichel, Dmitry Eremin-Solenikov, David Woodhouse,
Samuel Ortiz, Lee Jones, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA, linux-pm-u79uwXL29TY76Z2rM5mHXA
Cc: Liam Girdwood, Mark Brown, Rob Herring, Pawel Moll, Mark Rutland,
Ian Campbell, Kumar Gala, Kyungmin Park, Marek Szyprowski,
Bartlomiej Zolnierkiewicz, Krzysztof Kozlowski
Hi,
This is a fourth version of patches adding support for
MAX77836 device to the max14577 drivers.
This patchset has been on the lists for quite long time and was already
reviewed by some of the maintainers during previous submissions.
I am in a need of acks from power tree (patches: 3, 5-8).
Generally this should be pulled at once but some parts may be split
and pulled separately in following batches:
- Patches 1 and 2: as MFD prerequisites,
- Patches 3-5: adding actual charger code, depends on 1 and 2,
- Patch 6: fuel-gauge, independent,
- Patches 7 and 8: only documentation.
Lee Jones said he can take the set through his tree. Still I need acks
from power subsystem.
Everything rebased on current Linus' tree (v3.17-rc2-9-g68e370289c29).
Changes since v3
================
1. Split MFD changes into separate new patches:
1/8: mfd: max14577: Add defines for MAX77836 charger
2/8: Map charger device to its own of_node
Suggested by Lee Jones.
Changes since v2
================
1. charger: Use sysfs instead of DTS for setting the fast charge timer.
The charger driver now selects the CONFIG_SYSFS and exports
a DEVICE_ATTR. (suggested by Mark Rutland)
2. Add patch 6 with documentation of exported sysfs entry for fast
charge timer.
3. charger 3/6: Add missing 'break' in switch parsing valid values
for fast charge timer.
Changes since v1
================
1. charger 3/5: Add an error message for each unsuccessful parse of DT
property (suggested by Mark Rutland).
2. charger 3/5: Use 'u32' type for storing values from DT (suggested
by Mark Rutland).
3. charger 3/5: Remove an error message for memory allocation failure.
The patchset (first and second part of the MAX77836 drivers) has been
on the lists since January. Changelog for the first part of the drivers
(merged):
https://www.mail-archive.com/linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org/msg628696.html
Best regards,
Krzysztof Kozlowski
Krzysztof Kozlowski (8):
mfd: max14577: Add defines for MAX77836 charger
mfd: max14577: Map charger device to its own of_node
charger: max14577: Add support for MAX77836 charger
regulator/mfd: max14577: Export symbols for calculating charger
current
charger: max14577: Configure battery-dependent settings from DTS and
sysfs
power: max17040: Add ID for MAX77836 Fuel Gauge block
devicetree: mfd: max14577: Add device tree bindings document
Documentation: charger: max14577: Document exported sysfs entry
Documentation/ABI/testing/sysfs-class-power | 14 +
Documentation/devicetree/bindings/mfd/max14577.txt | 146 ++++++++
drivers/mfd/max14577.c | 100 +++++-
drivers/power/Kconfig | 5 +-
drivers/power/max14577_charger.c | 370 +++++++++++++++++++--
drivers/power/max17040_battery.c | 1 +
drivers/regulator/max14577.c | 80 +----
include/linux/mfd/max14577-private.h | 95 ++++--
include/linux/mfd/max14577.h | 30 ++
9 files changed, 703 insertions(+), 138 deletions(-)
create mode 100644 Documentation/devicetree/bindings/mfd/max14577.txt
--
1.9.1
^ permalink raw reply
* [PATCH v4 1/8] mfd: max14577: Add defines for MAX77836 charger
From: Krzysztof Kozlowski @ 2014-08-27 15:20 UTC (permalink / raw)
To: Sebastian Reichel, Dmitry Eremin-Solenikov, David Woodhouse,
Samuel Ortiz, Lee Jones, linux-kernel, linux-api, linux-pm
Cc: Liam Girdwood, Mark Brown, Rob Herring, Pawel Moll, Mark Rutland,
Ian Campbell, Kumar Gala, Kyungmin Park, Marek Szyprowski,
Bartlomiej Zolnierkiewicz, Krzysztof Kozlowski
In-Reply-To: <1409152814-17302-1-git-send-email-k.kozlowski@samsung.com>
Prepare for adding support for MAX77836 charger to the max14577 charger
driver by adding necessary new defines and prefixes to existing ones.
The MAX77836 uses slightly different values for ChgTyp field of STATUS2
register. On the MAX14577 value of 0x6 is reserved and 0x7 dead battery.
On the MAX77836 the opposite:
- 0x6 means special charger,
- 0x7 is reserved.
Regardless of these differences use one common enum
max14577_muic_charger_type.
Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Cc: Kyungmin Park <kyungmin.park@samsung.com>
---
include/linux/mfd/max14577-private.h | 54 ++++++++++++++++++++++++++----------
1 file changed, 40 insertions(+), 14 deletions(-)
diff --git a/include/linux/mfd/max14577-private.h b/include/linux/mfd/max14577-private.h
index 499253604026..d6f321699b89 100644
--- a/include/linux/mfd/max14577-private.h
+++ b/include/linux/mfd/max14577-private.h
@@ -72,15 +72,33 @@ enum max14577_muic_reg {
MAX14577_MUIC_REG_END,
};
+/*
+ * Combined charger types for max14577 and max77836.
+ *
+ * On max14577 three lower bits map to STATUS2/CHGTYP field.
+ * However the max77836 has different two last values of STATUS2/CHGTYP.
+ * To indicate the difference enum has two additional values for max77836.
+ * These values are just a register value bitwise OR with 0x8.
+ */
enum max14577_muic_charger_type {
- MAX14577_CHARGER_TYPE_NONE = 0,
- MAX14577_CHARGER_TYPE_USB,
- MAX14577_CHARGER_TYPE_DOWNSTREAM_PORT,
- MAX14577_CHARGER_TYPE_DEDICATED_CHG,
- MAX14577_CHARGER_TYPE_SPECIAL_500MA,
- MAX14577_CHARGER_TYPE_SPECIAL_1A,
- MAX14577_CHARGER_TYPE_RESERVED,
- MAX14577_CHARGER_TYPE_DEAD_BATTERY = 7,
+ MAX14577_CHARGER_TYPE_NONE = 0x0,
+ MAX14577_CHARGER_TYPE_USB = 0x1,
+ MAX14577_CHARGER_TYPE_DOWNSTREAM_PORT = 0x2,
+ MAX14577_CHARGER_TYPE_DEDICATED_CHG = 0x3,
+ MAX14577_CHARGER_TYPE_SPECIAL_500MA = 0x4,
+ /* Special 1A or 2A charger */
+ MAX14577_CHARGER_TYPE_SPECIAL_1A = 0x5,
+ /* max14577: reserved, used on max77836 */
+ MAX14577_CHARGER_TYPE_RESERVED = 0x6,
+ /* max14577: dead-battery charing with maximum current 100mA */
+ MAX14577_CHARGER_TYPE_DEAD_BATTERY = 0x7,
+ /*
+ * max77836: special charger (bias on D+/D-),
+ * matches register value of 0x6
+ */
+ MAX77836_CHARGER_TYPE_SPECIAL_BIAS = 0xe,
+ /* max77836: reserved, register value 0x7 */
+ MAX77836_CHARGER_TYPE_RESERVED = 0xf,
};
/* MAX14577 interrupts */
@@ -121,13 +139,15 @@ enum max14577_muic_charger_type {
#define STATUS2_CHGTYP_SHIFT 0
#define STATUS2_CHGDETRUN_SHIFT 3
#define STATUS2_DCDTMR_SHIFT 4
-#define STATUS2_DBCHG_SHIFT 5
+#define MAX14577_STATUS2_DBCHG_SHIFT 5
+#define MAX77836_STATUS2_DXOVP_SHIFT 5
#define STATUS2_VBVOLT_SHIFT 6
#define MAX77836_STATUS2_VIDRM_SHIFT 7
#define STATUS2_CHGTYP_MASK (0x7 << STATUS2_CHGTYP_SHIFT)
#define STATUS2_CHGDETRUN_MASK BIT(STATUS2_CHGDETRUN_SHIFT)
#define STATUS2_DCDTMR_MASK BIT(STATUS2_DCDTMR_SHIFT)
-#define STATUS2_DBCHG_MASK BIT(STATUS2_DBCHG_SHIFT)
+#define MAX14577_STATUS2_DBCHG_MASK BIT(MAX14577_STATUS2_DBCHG_SHIFT)
+#define MAX77836_STATUS2_DXOVP_MASK BIT(MAX77836_STATUS2_DXOVP_SHIFT)
#define STATUS2_VBVOLT_MASK BIT(STATUS2_VBVOLT_SHIFT)
#define MAX77836_STATUS2_VIDRM_MASK BIT(MAX77836_STATUS2_VIDRM_SHIFT)
@@ -177,9 +197,11 @@ enum max14577_muic_charger_type {
#define CTRL3_JIGSET_SHIFT 0
#define CTRL3_BOOTSET_SHIFT 2
#define CTRL3_ADCDBSET_SHIFT 4
+#define CTRL3_WBTH_SHIFT 6
#define CTRL3_JIGSET_MASK (0x3 << CTRL3_JIGSET_SHIFT)
#define CTRL3_BOOTSET_MASK (0x3 << CTRL3_BOOTSET_SHIFT)
#define CTRL3_ADCDBSET_MASK (0x3 << CTRL3_ADCDBSET_SHIFT)
+#define CTRL3_WBTH_MASK (0x3 << CTRL3_WBTH_SHIFT)
/* Slave addr = 0x4A: Charger */
enum max14577_charger_reg {
@@ -210,16 +232,20 @@ enum max14577_charger_reg {
#define CDETCTRL1_CHGTYPMAN_SHIFT 1
#define CDETCTRL1_DCDEN_SHIFT 2
#define CDETCTRL1_DCD2SCT_SHIFT 3
-#define CDETCTRL1_DCHKTM_SHIFT 4
-#define CDETCTRL1_DBEXIT_SHIFT 5
+#define MAX14577_CDETCTRL1_DCHKTM_SHIFT 4
+#define MAX77836_CDETCTRL1_CDLY_SHIFT 4
+#define MAX14577_CDETCTRL1_DBEXIT_SHIFT 5
+#define MAX77836_CDETCTRL1_DCDCPL_SHIFT 5
#define CDETCTRL1_DBIDLE_SHIFT 6
#define CDETCTRL1_CDPDET_SHIFT 7
#define CDETCTRL1_CHGDETEN_MASK BIT(CDETCTRL1_CHGDETEN_SHIFT)
#define CDETCTRL1_CHGTYPMAN_MASK BIT(CDETCTRL1_CHGTYPMAN_SHIFT)
#define CDETCTRL1_DCDEN_MASK BIT(CDETCTRL1_DCDEN_SHIFT)
#define CDETCTRL1_DCD2SCT_MASK BIT(CDETCTRL1_DCD2SCT_SHIFT)
-#define CDETCTRL1_DCHKTM_MASK BIT(CDETCTRL1_DCHKTM_SHIFT)
-#define CDETCTRL1_DBEXIT_MASK BIT(CDETCTRL1_DBEXIT_SHIFT)
+#define MAX14577_CDETCTRL1_DCHKTM_MASK BIT(MAX14577_CDETCTRL1_DCHKTM_SHIFT)
+#define MAX77836_CDETCTRL1_CDDLY_MASK BIT(MAX77836_CDETCTRL1_CDDLY_SHIFT)
+#define MAX14577_CDETCTRL1_DBEXIT_MASK BIT(MAX14577_CDETCTRL1_DBEXIT_SHIFT)
+#define MAX77836_CDETCTRL1_DCDCPL_MASK BIT(MAX77836_CDETCTRL1_DCDCPL_SHIFT)
#define CDETCTRL1_DBIDLE_MASK BIT(CDETCTRL1_DBIDLE_SHIFT)
#define CDETCTRL1_CDPDET_MASK BIT(CDETCTRL1_CDPDET_SHIFT)
--
1.9.1
^ permalink raw reply related
* [PATCH v4 2/8] mfd: max14577: Map charger device to its own of_node
From: Krzysztof Kozlowski @ 2014-08-27 15:20 UTC (permalink / raw)
To: Sebastian Reichel, Dmitry Eremin-Solenikov, David Woodhouse,
Samuel Ortiz, Lee Jones, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA, linux-pm-u79uwXL29TY76Z2rM5mHXA
Cc: Liam Girdwood, Mark Brown, Rob Herring, Pawel Moll, Mark Rutland,
Ian Campbell, Kumar Gala, Kyungmin Park, Marek Szyprowski,
Bartlomiej Zolnierkiewicz, Krzysztof Kozlowski
In-Reply-To: <1409152814-17302-1-git-send-email-k.kozlowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
Add a "maxim,max14577-charger" of_compatible to the mfd_cell so the
MFD child device (the charger) will have its own of_node set. This will
be used by the max14577 charger driver in next patches to obtain battery
configuration from DTS.
Signed-off-by: Krzysztof Kozlowski <k.kozlowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
Cc: Kyungmin Park <kyungmin.park-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
---
drivers/mfd/max14577.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/mfd/max14577.c b/drivers/mfd/max14577.c
index 4a5e885383f8..6599407b5624 100644
--- a/drivers/mfd/max14577.c
+++ b/drivers/mfd/max14577.c
@@ -35,7 +35,10 @@ static const struct mfd_cell max14577_devs[] = {
.name = "max14577-regulator",
.of_compatible = "maxim,max14577-regulator",
},
- { .name = "max14577-charger", },
+ {
+ .name = "max14577-charger",
+ .of_compatible = "maxim,max14577-charger",
+ },
};
static const struct mfd_cell max77836_devs[] = {
--
1.9.1
^ permalink raw reply related
* [PATCH v4 3/8] charger: max14577: Add support for MAX77836 charger
From: Krzysztof Kozlowski @ 2014-08-27 15:20 UTC (permalink / raw)
To: Sebastian Reichel, Dmitry Eremin-Solenikov, David Woodhouse,
Samuel Ortiz, Lee Jones, linux-kernel, linux-api, linux-pm
Cc: Liam Girdwood, Mark Brown, Rob Herring, Pawel Moll, Mark Rutland,
Ian Campbell, Kumar Gala, Kyungmin Park, Marek Szyprowski,
Bartlomiej Zolnierkiewicz, Krzysztof Kozlowski
In-Reply-To: <1409152814-17302-1-git-send-email-k.kozlowski@samsung.com>
Add support for MAX77836 charger to the max14577 driver. The MAX77836
charger is almost the same as 14577 model except:
- No dead-battery detection;
- Support for special charger (like in MAX77693);
- Support for DX over-voltage protection (like in MAX77693);
- Lower values of charging current (two times lower current for
slow/fast charge, much lower EOC current);
- Slightly different values in ChgTyp field of STATUS2 register. On
MAX14577 0x6 is reserved and 0x7 dead battery. On the MAX77836 the
0x6 means special charger and 0x7 is reserved. Regardless of these
differences the driver maps them to one enum max14577_muic_charger_type.
Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Cc: Kyungmin Park <kyungmin.park@samsung.com>
---
drivers/power/Kconfig | 4 +--
drivers/power/max14577_charger.c | 77 ++++++++++++++++++++++++++++++++--------
2 files changed, 64 insertions(+), 17 deletions(-)
diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig
index 73cfcdf28a36..69fa8a9ef7a6 100644
--- a/drivers/power/Kconfig
+++ b/drivers/power/Kconfig
@@ -325,11 +325,11 @@ config CHARGER_MANAGER
with help of suspend_again support.
config CHARGER_MAX14577
- tristate "Maxim MAX14577 MUIC battery charger driver"
+ tristate "Maxim MAX14577/77836 battery charger driver"
depends on MFD_MAX14577
help
Say Y to enable support for the battery charger control sysfs and
- platform data of MAX14577 MUICs.
+ platform data of MAX14577/77836 MUICs.
config CHARGER_MAX8997
tristate "Maxim MAX8997/MAX8966 PMIC battery charger driver"
diff --git a/drivers/power/max14577_charger.c b/drivers/power/max14577_charger.c
index fad2a75b3604..19c8f42abf24 100644
--- a/drivers/power/max14577_charger.c
+++ b/drivers/power/max14577_charger.c
@@ -1,7 +1,7 @@
/*
- * Battery charger driver for the Maxim 14577
+ * max14577_charger.c - Battery charger driver for the Maxim 14577/77836
*
- * Copyright (C) 2013 Samsung Electronics
+ * Copyright (C) 2013,2014 Samsung Electronics
* Krzysztof Kozlowski <k.kozlowski@samsung.com>
*
* This program is free software; you can redistribute it and/or modify
@@ -25,10 +25,35 @@ struct max14577_charger {
struct max14577 *max14577;
struct power_supply charger;
- unsigned int charging_state;
- unsigned int battery_state;
+ unsigned int charging_state;
+ unsigned int battery_state;
};
+/*
+ * Helper function for mapping values of STATUS2/CHGTYP register on max14577
+ * and max77836 chipsets to enum maxim_muic_charger_type.
+ */
+static enum max14577_muic_charger_type maxim_get_charger_type(
+ enum maxim_device_type dev_type, u8 val) {
+ switch (val) {
+ case MAX14577_CHARGER_TYPE_NONE:
+ case MAX14577_CHARGER_TYPE_USB:
+ case MAX14577_CHARGER_TYPE_DOWNSTREAM_PORT:
+ case MAX14577_CHARGER_TYPE_DEDICATED_CHG:
+ case MAX14577_CHARGER_TYPE_SPECIAL_500MA:
+ case MAX14577_CHARGER_TYPE_SPECIAL_1A:
+ return val;
+ case MAX14577_CHARGER_TYPE_DEAD_BATTERY:
+ case MAX14577_CHARGER_TYPE_RESERVED:
+ if (dev_type == MAXIM_DEVICE_TYPE_MAX77836)
+ val |= 0x8;
+ return val;
+ default:
+ WARN_ONCE(1, "max14577: Unsupported chgtyp register value 0x%02x", val);
+ return val;
+ }
+}
+
static int max14577_get_charger_state(struct max14577_charger *chg)
{
struct regmap *rmap = chg->max14577->regmap;
@@ -89,19 +114,23 @@ static int max14577_get_online(struct max14577_charger *chg)
{
struct regmap *rmap = chg->max14577->regmap;
u8 reg_data;
+ enum max14577_muic_charger_type chg_type;
max14577_read_reg(rmap, MAX14577_MUIC_REG_STATUS2, ®_data);
reg_data = ((reg_data & STATUS2_CHGTYP_MASK) >> STATUS2_CHGTYP_SHIFT);
- switch (reg_data) {
+ chg_type = maxim_get_charger_type(chg->max14577->dev_type, reg_data);
+ switch (chg_type) {
case MAX14577_CHARGER_TYPE_USB:
case MAX14577_CHARGER_TYPE_DEDICATED_CHG:
case MAX14577_CHARGER_TYPE_SPECIAL_500MA:
case MAX14577_CHARGER_TYPE_SPECIAL_1A:
case MAX14577_CHARGER_TYPE_DEAD_BATTERY:
+ case MAX77836_CHARGER_TYPE_SPECIAL_BIAS:
return 1;
case MAX14577_CHARGER_TYPE_NONE:
case MAX14577_CHARGER_TYPE_DOWNSTREAM_PORT:
case MAX14577_CHARGER_TYPE_RESERVED:
+ case MAX77836_CHARGER_TYPE_RESERVED:
default:
return 0;
}
@@ -118,10 +147,12 @@ static int max14577_get_battery_health(struct max14577_charger *chg)
struct regmap *rmap = chg->max14577->regmap;
int state = POWER_SUPPLY_HEALTH_GOOD;
u8 reg_data;
+ enum max14577_muic_charger_type chg_type;
max14577_read_reg(rmap, MAX14577_MUIC_REG_STATUS2, ®_data);
reg_data = ((reg_data & STATUS2_CHGTYP_MASK) >> STATUS2_CHGTYP_SHIFT);
- if (reg_data == MAX14577_CHARGER_TYPE_DEAD_BATTERY) {
+ chg_type = maxim_get_charger_type(chg->max14577->dev_type, reg_data);
+ if (chg_type == MAX14577_CHARGER_TYPE_DEAD_BATTERY) {
state = POWER_SUPPLY_HEALTH_DEAD;
goto state_set;
}
@@ -167,7 +198,7 @@ static void max14577_charger_reg_init(struct max14577_charger *chg)
CDETCTRL1_CHGDETEN_MASK | CDETCTRL1_CHGTYPMAN_MASK,
reg_data);
- /* Battery Fast-Charge Timer, from SM-V700: 6hrs */
+ /* Battery Fast-Charge Timer, set to: 6hrs */
reg_data = 0x3 << CHGCTRL1_TCHW_SHIFT;
max14577_write_reg(rmap, MAX14577_REG_CHGCTRL1, reg_data);
@@ -179,19 +210,22 @@ static void max14577_charger_reg_init(struct max14577_charger *chg)
reg_data |= 0x1 << CHGCTRL2_MBCHOSTEN_SHIFT;
max14577_write_reg(rmap, MAX14577_REG_CHGCTRL2, reg_data);
- /* Battery-Charger Constant Voltage (CV) Mode, from SM-V700: 4.35V */
+ /* Battery-Charger Constant Voltage (CV) Mode, set to: 4.35V */
reg_data = 0xf << CHGCTRL3_MBCCVWRC_SHIFT;
max14577_write_reg(rmap, MAX14577_REG_CHGCTRL3, reg_data);
/*
- * Fast Battery-Charge Current Low, default 200-950mA
- * Fast Battery-Charge Current High, from SM-V700: 450mA
+ * Fast Battery-Charge Current Low,
+ * default 200-950mA (max14577) / 100-475mA (max77836)
+ *
+ * Fast Battery-Charge Current High,
+ * set to 450mA (max14577) / 225mA (max77836)
*/
reg_data = 0x1 << CHGCTRL4_MBCICHWRCL_SHIFT;
reg_data |= 0x5 << CHGCTRL4_MBCICHWRCH_SHIFT;
max14577_write_reg(rmap, MAX14577_REG_CHGCTRL4, reg_data);
- /* End-of-Charge Current, from SM-V700: 50mA */
+ /* End-of-Charge Current, set to 50mA (max14577) / 7.5mA (max77836) */
reg_data = 0x0 << CHGCTRL5_EOCS_SHIFT;
max14577_write_reg(rmap, MAX14577_REG_CHGCTRL5, reg_data);
@@ -199,7 +233,7 @@ static void max14577_charger_reg_init(struct max14577_charger *chg)
reg_data = 0x0 << CHGCTRL6_AUTOSTOP_SHIFT;
max14577_write_reg(rmap, MAX14577_REG_CHGCTRL6, reg_data);
- /* Overvoltage-Protection Threshold, from SM-V700: 6.5V */
+ /* Overvoltage-Protection Threshold, set to 6.5V */
reg_data = 0x2 << CHGCTRL7_OTPCGHCVS_SHIFT;
max14577_write_reg(rmap, MAX14577_REG_CHGCTRL7, reg_data);
}
@@ -215,7 +249,11 @@ static enum power_supply_property max14577_charger_props[] = {
POWER_SUPPLY_PROP_MANUFACTURER,
};
-static const char *model_name = "MAX14577";
+static const char * const model_names[] = {
+ [MAXIM_DEVICE_TYPE_UNKNOWN] = "MAX14577-like",
+ [MAXIM_DEVICE_TYPE_MAX14577] = "MAX14577",
+ [MAXIM_DEVICE_TYPE_MAX77836] = "MAX77836",
+};
static const char *manufacturer = "Maxim Integrated";
static int max14577_charger_get_property(struct power_supply *psy,
@@ -244,7 +282,8 @@ static int max14577_charger_get_property(struct power_supply *psy,
val->intval = max14577_get_online(chg);
break;
case POWER_SUPPLY_PROP_MODEL_NAME:
- val->strval = model_name;
+ BUILD_BUG_ON(ARRAY_SIZE(model_names) != MAXIM_DEVICE_TYPE_NUM);
+ val->strval = model_names[chg->max14577->dev_type];
break;
case POWER_SUPPLY_PROP_MANUFACTURER:
val->strval = manufacturer;
@@ -296,6 +335,13 @@ static int max14577_charger_remove(struct platform_device *pdev)
return 0;
}
+static const struct platform_device_id max14577_charger_id[] = {
+ { "max14577-charger", MAXIM_DEVICE_TYPE_MAX14577, },
+ { "max77836-charger", MAXIM_DEVICE_TYPE_MAX77836, },
+ { }
+};
+MODULE_DEVICE_TABLE(platform, max14577_regulator_id);
+
static struct platform_driver max14577_charger_driver = {
.driver = {
.owner = THIS_MODULE,
@@ -303,9 +349,10 @@ static struct platform_driver max14577_charger_driver = {
},
.probe = max14577_charger_probe,
.remove = max14577_charger_remove,
+ .id_table = max14577_charger_id,
};
module_platform_driver(max14577_charger_driver);
MODULE_AUTHOR("Krzysztof Kozlowski <k.kozlowski@samsung.com>");
-MODULE_DESCRIPTION("MAXIM 14577 charger driver");
+MODULE_DESCRIPTION("Maxim 14577/77836 charger driver");
MODULE_LICENSE("GPL");
--
1.9.1
^ permalink raw reply related
* [PATCH v4 4/8] regulator/mfd: max14577: Export symbols for calculating charger current
From: Krzysztof Kozlowski @ 2014-08-27 15:20 UTC (permalink / raw)
To: Sebastian Reichel, Dmitry Eremin-Solenikov, David Woodhouse,
Samuel Ortiz, Lee Jones, linux-kernel, linux-api, linux-pm
Cc: Liam Girdwood, Mark Brown, Rob Herring, Pawel Moll, Mark Rutland,
Ian Campbell, Kumar Gala, Kyungmin Park, Marek Szyprowski,
Bartlomiej Zolnierkiewicz, Krzysztof Kozlowski
In-Reply-To: <1409152814-17302-1-git-send-email-k.kozlowski@samsung.com>
This patch prepares for changing the max14577 charger driver to allow
configuring battery-dependent settings from DTS.
The patch moves from regulator driver to MFD core driver and exports:
- function for calculating register value for charger's current;
- table of limits for chargers (MAX14577, MAX77836).
Previously they were used only by the max14577 regulator driver. In next
patch the charger driver will use them as well. Exporting them will
reduce unnecessary code duplication.
Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Cc: Kyungmin Park <kyungmin.park@samsung.com>
Acked-by: Mark Brown <broonie@linaro.org>
Acked-by: Lee Jones <lee.jones@linaro.org>
---
drivers/mfd/max14577.c | 95 ++++++++++++++++++++++++++++++++++++
drivers/regulator/max14577.c | 80 ++----------------------------
include/linux/mfd/max14577-private.h | 22 ++++-----
include/linux/mfd/max14577.h | 23 +++++++++
4 files changed, 133 insertions(+), 87 deletions(-)
diff --git a/drivers/mfd/max14577.c b/drivers/mfd/max14577.c
index 6599407b5624..b8af263be594 100644
--- a/drivers/mfd/max14577.c
+++ b/drivers/mfd/max14577.c
@@ -26,6 +26,87 @@
#include <linux/mfd/max14577.h>
#include <linux/mfd/max14577-private.h>
+/*
+ * Table of valid charger currents for different Maxim chipsets.
+ * It is placed here because it is used by both charger and regulator driver.
+ */
+const struct maxim_charger_current maxim_charger_currents[] = {
+ [MAXIM_DEVICE_TYPE_UNKNOWN] = { 0, 0, 0, 0 },
+ [MAXIM_DEVICE_TYPE_MAX14577] = {
+ .min = MAX14577_CHARGER_CURRENT_LIMIT_MIN,
+ .high_start = MAX14577_CHARGER_CURRENT_LIMIT_HIGH_START,
+ .high_step = MAX14577_CHARGER_CURRENT_LIMIT_HIGH_STEP,
+ .max = MAX14577_CHARGER_CURRENT_LIMIT_MAX,
+ },
+ [MAXIM_DEVICE_TYPE_MAX77836] = {
+ .min = MAX77836_CHARGER_CURRENT_LIMIT_MIN,
+ .high_start = MAX77836_CHARGER_CURRENT_LIMIT_HIGH_START,
+ .high_step = MAX77836_CHARGER_CURRENT_LIMIT_HIGH_STEP,
+ .max = MAX77836_CHARGER_CURRENT_LIMIT_MAX,
+ },
+};
+EXPORT_SYMBOL_GPL(maxim_charger_currents);
+
+/*
+ * maxim_charger_calc_reg_current - Calculate register value for current
+ * @limits: constraints for charger, matching the MBCICHWRC register
+ * @min_ua: minimal requested current, micro Amps
+ * @max_ua: maximum requested current, micro Amps
+ * @dst: destination to store calculated register value
+ *
+ * Calculates the value of MBCICHWRC (Fast Battery Charge Current) register
+ * for given current and stores it under pointed 'dst'. The stored value
+ * combines low bit (MBCICHWRCL) and high bits (MBCICHWRCH). It is also
+ * properly shifted.
+ *
+ * The calculated register value matches the current which:
+ * - is always between <limits.min, limits.max>;
+ * - is always less or equal to max_ua;
+ * - is the highest possible value;
+ * - may be lower than min_ua.
+ *
+ * On success returns 0. On error returns -EINVAL (requested min/max current
+ * is outside of given charger limits) and 'dst' is not set.
+ */
+int maxim_charger_calc_reg_current(const struct maxim_charger_current *limits,
+ unsigned int min_ua, unsigned int max_ua, u8 *dst)
+{
+ unsigned int current_bits = 0xf;
+
+ if (min_ua > max_ua)
+ return -EINVAL;
+
+ if (min_ua > limits->max || max_ua < limits->min)
+ return -EINVAL;
+
+ if (max_ua < limits->high_start) {
+ /*
+ * Less than high_start, so set the minimal current
+ * (turn Low Bit off, 0 as high bits).
+ */
+ *dst = 0x0;
+ return 0;
+ }
+
+ /* max_ua is in range: <high_start, infinite>, cut it to limits.max */
+ max_ua = min(limits->max, max_ua);
+ max_ua -= limits->high_start;
+ /*
+ * There is no risk of overflow 'max_ua' here because:
+ * - max_ua >= limits.high_start
+ * - BUILD_BUG checks that 'limits' are: max >= high_start + high_step
+ */
+ current_bits = max_ua / limits->high_step;
+
+ /* Turn Low Bit on (use range <limits.high_start, limits.max>) ... */
+ *dst = 0x1 << CHGCTRL4_MBCICHWRCL_SHIFT;
+ /* and set proper High Bits */
+ *dst |= current_bits << CHGCTRL4_MBCICHWRCH_SHIFT;
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(maxim_charger_calc_reg_current);
+
static const struct mfd_cell max14577_devs[] = {
{
.name = "max14577-muic",
@@ -466,6 +547,20 @@ static int __init max14577_i2c_init(void)
BUILD_BUG_ON(ARRAY_SIZE(max14577_i2c_id) != MAXIM_DEVICE_TYPE_NUM);
BUILD_BUG_ON(ARRAY_SIZE(max14577_dt_match) != MAXIM_DEVICE_TYPE_NUM);
+ /* Valid charger current values must be provided for each chipset */
+ BUILD_BUG_ON(ARRAY_SIZE(maxim_charger_currents) != MAXIM_DEVICE_TYPE_NUM);
+
+ /* Check for valid values for charger */
+ BUILD_BUG_ON(MAX14577_CHARGER_CURRENT_LIMIT_HIGH_START +
+ MAX14577_CHARGER_CURRENT_LIMIT_HIGH_STEP * 0xf !=
+ MAX14577_CHARGER_CURRENT_LIMIT_MAX);
+ BUILD_BUG_ON(MAX14577_CHARGER_CURRENT_LIMIT_HIGH_STEP == 0);
+
+ BUILD_BUG_ON(MAX77836_CHARGER_CURRENT_LIMIT_HIGH_START +
+ MAX77836_CHARGER_CURRENT_LIMIT_HIGH_STEP * 0xf !=
+ MAX77836_CHARGER_CURRENT_LIMIT_MAX);
+ BUILD_BUG_ON(MAX77836_CHARGER_CURRENT_LIMIT_HIGH_STEP == 0);
+
return i2c_add_driver(&max14577_i2c_driver);
}
subsys_initcall(max14577_i2c_init);
diff --git a/drivers/regulator/max14577.c b/drivers/regulator/max14577.c
index 5d9c605cf534..0ff5a20ac958 100644
--- a/drivers/regulator/max14577.c
+++ b/drivers/regulator/max14577.c
@@ -22,42 +22,6 @@
#include <linux/mfd/max14577-private.h>
#include <linux/regulator/of_regulator.h>
-/*
- * Valid limits of current for max14577 and max77836 chargers.
- * They must correspond to MBCICHWRCL and MBCICHWRCH fields in CHGCTRL4
- * register for given chipset.
- */
-struct maxim_charger_current {
- /* Minimal current, set in CHGCTRL4/MBCICHWRCL, uA */
- unsigned int min;
- /*
- * Minimal current when high setting is active,
- * set in CHGCTRL4/MBCICHWRCH, uA
- */
- unsigned int high_start;
- /* Value of one step in high setting, uA */
- unsigned int high_step;
- /* Maximum current of high setting, uA */
- unsigned int max;
-};
-
-/* Table of valid charger currents for different Maxim chipsets */
-static const struct maxim_charger_current maxim_charger_currents[] = {
- [MAXIM_DEVICE_TYPE_UNKNOWN] = { 0, 0, 0, 0 },
- [MAXIM_DEVICE_TYPE_MAX14577] = {
- .min = MAX14577_REGULATOR_CURRENT_LIMIT_MIN,
- .high_start = MAX14577_REGULATOR_CURRENT_LIMIT_HIGH_START,
- .high_step = MAX14577_REGULATOR_CURRENT_LIMIT_HIGH_STEP,
- .max = MAX14577_REGULATOR_CURRENT_LIMIT_MAX,
- },
- [MAXIM_DEVICE_TYPE_MAX77836] = {
- .min = MAX77836_REGULATOR_CURRENT_LIMIT_MIN,
- .high_start = MAX77836_REGULATOR_CURRENT_LIMIT_HIGH_START,
- .high_step = MAX77836_REGULATOR_CURRENT_LIMIT_HIGH_STEP,
- .max = MAX77836_REGULATOR_CURRENT_LIMIT_MAX,
- },
-};
-
static int max14577_reg_is_enabled(struct regulator_dev *rdev)
{
int rid = rdev_get_id(rdev);
@@ -103,8 +67,8 @@ static int max14577_reg_get_current_limit(struct regulator_dev *rdev)
static int max14577_reg_set_current_limit(struct regulator_dev *rdev,
int min_uA, int max_uA)
{
- int i, current_bits = 0xf;
u8 reg_data;
+ int ret;
struct max14577 *max14577 = rdev_get_drvdata(rdev);
const struct maxim_charger_current *limits =
&maxim_charger_currents[max14577->dev_type];
@@ -112,35 +76,9 @@ static int max14577_reg_set_current_limit(struct regulator_dev *rdev,
if (rdev_get_id(rdev) != MAX14577_CHARGER)
return -EINVAL;
- if (min_uA > limits->max || max_uA < limits->min)
- return -EINVAL;
-
- if (max_uA < limits->high_start) {
- /*
- * Less than high_start,
- * so set the minimal current (turn only Low Bit off)
- */
- u8 reg_data = 0x0 << CHGCTRL4_MBCICHWRCL_SHIFT;
- return max14577_update_reg(rdev->regmap,
- MAX14577_CHG_REG_CHG_CTRL4,
- CHGCTRL4_MBCICHWRCL_MASK, reg_data);
- }
-
- /*
- * max_uA is in range: <high_start, inifinite>, so search for
- * valid current starting from maximum current.
- */
- for (i = limits->max; i >= limits->high_start; i -= limits->high_step) {
- if (i <= max_uA)
- break;
- current_bits--;
- }
- BUG_ON(current_bits < 0); /* Cannot happen */
-
- /* Turn Low Bit on (use range high_start-max)... */
- reg_data = 0x1 << CHGCTRL4_MBCICHWRCL_SHIFT;
- /* and set proper High Bits */
- reg_data |= current_bits << CHGCTRL4_MBCICHWRCH_SHIFT;
+ ret = maxim_charger_calc_reg_current(limits, min_uA, max_uA, ®_data);
+ if (ret)
+ return ret;
return max14577_update_reg(rdev->regmap, MAX14577_CHG_REG_CHG_CTRL4,
CHGCTRL4_MBCICHWRCL_MASK | CHGCTRL4_MBCICHWRCH_MASK,
@@ -442,16 +380,6 @@ static struct platform_driver max14577_regulator_driver = {
static int __init max14577_regulator_init(void)
{
- /* Check for valid values for charger */
- BUILD_BUG_ON(MAX14577_REGULATOR_CURRENT_LIMIT_HIGH_START +
- MAX14577_REGULATOR_CURRENT_LIMIT_HIGH_STEP * 0xf !=
- MAX14577_REGULATOR_CURRENT_LIMIT_MAX);
- BUILD_BUG_ON(MAX77836_REGULATOR_CURRENT_LIMIT_HIGH_START +
- MAX77836_REGULATOR_CURRENT_LIMIT_HIGH_STEP * 0xf !=
- MAX77836_REGULATOR_CURRENT_LIMIT_MAX);
- /* Valid charger current values must be provided for each chipset */
- BUILD_BUG_ON(ARRAY_SIZE(maxim_charger_currents) != MAXIM_DEVICE_TYPE_NUM);
-
BUILD_BUG_ON(ARRAY_SIZE(max14577_supported_regulators) != MAX14577_REGULATOR_NUM);
BUILD_BUG_ON(ARRAY_SIZE(max77836_supported_regulators) != MAX77836_REGULATOR_NUM);
diff --git a/include/linux/mfd/max14577-private.h b/include/linux/mfd/max14577-private.h
index d6f321699b89..7d514839c764 100644
--- a/include/linux/mfd/max14577-private.h
+++ b/include/linux/mfd/max14577-private.h
@@ -281,17 +281,17 @@ enum max14577_charger_reg {
#define CHGCTRL7_OTPCGHCVS_SHIFT 0
#define CHGCTRL7_OTPCGHCVS_MASK (0x3 << CHGCTRL7_OTPCGHCVS_SHIFT)
-/* MAX14577 regulator current limits (as in CHGCTRL4 register), uA */
-#define MAX14577_REGULATOR_CURRENT_LIMIT_MIN 90000
-#define MAX14577_REGULATOR_CURRENT_LIMIT_HIGH_START 200000
-#define MAX14577_REGULATOR_CURRENT_LIMIT_HIGH_STEP 50000
-#define MAX14577_REGULATOR_CURRENT_LIMIT_MAX 950000
-
-/* MAX77836 regulator current limits (as in CHGCTRL4 register), uA */
-#define MAX77836_REGULATOR_CURRENT_LIMIT_MIN 45000
-#define MAX77836_REGULATOR_CURRENT_LIMIT_HIGH_START 100000
-#define MAX77836_REGULATOR_CURRENT_LIMIT_HIGH_STEP 25000
-#define MAX77836_REGULATOR_CURRENT_LIMIT_MAX 475000
+/* MAX14577 charger current limits (as in CHGCTRL4 register), uA */
+#define MAX14577_CHARGER_CURRENT_LIMIT_MIN 90000U
+#define MAX14577_CHARGER_CURRENT_LIMIT_HIGH_START 200000U
+#define MAX14577_CHARGER_CURRENT_LIMIT_HIGH_STEP 50000U
+#define MAX14577_CHARGER_CURRENT_LIMIT_MAX 950000U
+
+/* MAX77836 charger current limits (as in CHGCTRL4 register), uA */
+#define MAX77836_CHARGER_CURRENT_LIMIT_MIN 45000U
+#define MAX77836_CHARGER_CURRENT_LIMIT_HIGH_START 100000U
+#define MAX77836_CHARGER_CURRENT_LIMIT_HIGH_STEP 25000U
+#define MAX77836_CHARGER_CURRENT_LIMIT_MAX 475000U
/* MAX14577 regulator SFOUT LDO voltage, fixed, uV */
#define MAX14577_REGULATOR_SAFEOUT_VOLTAGE 4900000
diff --git a/include/linux/mfd/max14577.h b/include/linux/mfd/max14577.h
index c83fbed1c7b6..3c098d57b1d1 100644
--- a/include/linux/mfd/max14577.h
+++ b/include/linux/mfd/max14577.h
@@ -74,4 +74,27 @@ struct max14577_platform_data {
struct max14577_regulator_platform_data *regulators;
};
+/*
+ * Valid limits of current for max14577 and max77836 chargers.
+ * They must correspond to MBCICHWRCL and MBCICHWRCH fields in CHGCTRL4
+ * register for given chipset.
+ */
+struct maxim_charger_current {
+ /* Minimal current, set in CHGCTRL4/MBCICHWRCL, uA */
+ unsigned int min;
+ /*
+ * Minimal current when high setting is active,
+ * set in CHGCTRL4/MBCICHWRCH, uA
+ */
+ unsigned int high_start;
+ /* Value of one step in high setting, uA */
+ unsigned int high_step;
+ /* Maximum current of high setting, uA */
+ unsigned int max;
+};
+
+extern const struct maxim_charger_current maxim_charger_currents[];
+extern int maxim_charger_calc_reg_current(const struct maxim_charger_current *limits,
+ unsigned int min_ua, unsigned int max_ua, u8 *dst);
+
#endif /* __MAX14577_H__ */
--
1.9.1
^ permalink raw reply related
* [PATCH v4 5/8] charger: max14577: Configure battery-dependent settings from DTS and sysfs
From: Krzysztof Kozlowski @ 2014-08-27 15:20 UTC (permalink / raw)
To: Sebastian Reichel, Dmitry Eremin-Solenikov, David Woodhouse,
Samuel Ortiz, Lee Jones, linux-kernel, linux-api, linux-pm
Cc: Liam Girdwood, Mark Brown, Rob Herring, Pawel Moll, Mark Rutland,
Ian Campbell, Kumar Gala, Kyungmin Park, Marek Szyprowski,
Bartlomiej Zolnierkiewicz, Krzysztof Kozlowski
In-Reply-To: <1409152814-17302-1-git-send-email-k.kozlowski@samsung.com>
Remove hard-coded values for:
- Fast Charge current,
- End Of Charge current,
- Fast Charge timer,
- Overvoltage Protection Threshold,
- Battery Constant Voltage,
and use DTS or sysfs to configure them. This allows using the max14577 charger
driver with different batteries.
Now the charger driver requires valid configuration data from DTS. In
case of wrong configuration data it fails during probe.
The fast charge timer is configured through sysfs entry.
Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Cc: Kyungmin Park <kyungmin.park@samsung.com>
Acked-by: Lee Jones <lee.jones@linaro.org>
---
drivers/power/Kconfig | 1 +
drivers/power/max14577_charger.c | 311 +++++++++++++++++++++++++++++++----
include/linux/mfd/max14577-private.h | 19 +++
include/linux/mfd/max14577.h | 7 +
4 files changed, 310 insertions(+), 28 deletions(-)
diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig
index 69fa8a9ef7a6..04e1d2fe2201 100644
--- a/drivers/power/Kconfig
+++ b/drivers/power/Kconfig
@@ -327,6 +327,7 @@ config CHARGER_MANAGER
config CHARGER_MAX14577
tristate "Maxim MAX14577/77836 battery charger driver"
depends on MFD_MAX14577
+ select SYSFS
help
Say Y to enable support for the battery charger control sysfs and
platform data of MAX14577/77836 MUICs.
diff --git a/drivers/power/max14577_charger.c b/drivers/power/max14577_charger.c
index 19c8f42abf24..c125756eab69 100644
--- a/drivers/power/max14577_charger.c
+++ b/drivers/power/max14577_charger.c
@@ -19,6 +19,7 @@
#include <linux/platform_device.h>
#include <linux/power_supply.h>
#include <linux/mfd/max14577-private.h>
+#include <linux/mfd/max14577.h>
struct max14577_charger {
struct device *dev;
@@ -27,6 +28,8 @@ struct max14577_charger {
unsigned int charging_state;
unsigned int battery_state;
+
+ struct max14577_charger_platform_data *pdata;
};
/*
@@ -178,15 +181,131 @@ static int max14577_get_present(struct max14577_charger *chg)
return 1;
}
+static inline int max14577_set_fast_charge_timer(struct max14577_charger *chg,
+ unsigned long hours)
+{
+ u8 reg_data;
+
+ switch (hours) {
+ case 5 ... 7:
+ reg_data = hours - 3;
+ break;
+ case 0:
+ /* Disable */
+ reg_data = 0x7;
+ break;
+ default:
+ dev_err(chg->dev, "Wrong value for Fast-Charge Timer: %lu\n",
+ hours);
+ return -EINVAL;
+ }
+ reg_data <<= CHGCTRL1_TCHW_SHIFT;
+
+ return max14577_update_reg(chg->max14577->regmap,
+ MAX14577_REG_CHGCTRL1, CHGCTRL1_TCHW_MASK, reg_data);
+}
+
+static inline int max14577_init_constant_voltage(struct max14577_charger *chg,
+ unsigned int uvolt)
+{
+ u8 reg_data;
+
+ if (uvolt < MAXIM_CHARGER_CONSTANT_VOLTAGE_MIN ||
+ uvolt > MAXIM_CHARGER_CONSTANT_VOLTAGE_MAX)
+ return -EINVAL;
+
+ if (uvolt == 4200000)
+ reg_data = 0x0;
+ else if (uvolt == MAXIM_CHARGER_CONSTANT_VOLTAGE_MAX)
+ reg_data = 0x1f;
+ else if (uvolt <= 4280000) {
+ unsigned int val = uvolt;
+
+ val -= MAXIM_CHARGER_CONSTANT_VOLTAGE_MIN;
+ val /= MAXIM_CHARGER_CONSTANT_VOLTAGE_STEP;
+ if (uvolt <= 4180000)
+ reg_data = 0x1 + val;
+ else
+ reg_data = val; /* Fix for gap between 4.18V and 4.22V */
+ } else
+ return -EINVAL;
+
+ reg_data <<= CHGCTRL3_MBCCVWRC_SHIFT;
+
+ return max14577_write_reg(chg->max14577->regmap,
+ MAX14577_CHG_REG_CHG_CTRL3, reg_data);
+}
+
+static inline int max14577_init_eoc(struct max14577_charger *chg,
+ unsigned int uamp)
+{
+ unsigned int current_bits = 0xf;
+ u8 reg_data;
+
+ switch (chg->max14577->dev_type) {
+ case MAXIM_DEVICE_TYPE_MAX77836:
+ if (uamp < 5000)
+ return -EINVAL; /* Requested current is too low */
+
+ if (uamp >= 7500 && uamp < 10000)
+ current_bits = 0x0;
+ else if (uamp <= 50000) {
+ /* <5000, 7499> and <10000, 50000> */
+ current_bits = uamp / 5000;
+ } else {
+ uamp = min(uamp, 100000U) - 50000U;
+ current_bits = 0xa + uamp / 10000;
+ }
+ break;
+
+ case MAXIM_DEVICE_TYPE_MAX14577:
+ default:
+ if (uamp < MAX14577_CHARGER_EOC_CURRENT_LIMIT_MIN)
+ return -EINVAL; /* Requested current is too low */
+
+ uamp = min(uamp, MAX14577_CHARGER_EOC_CURRENT_LIMIT_MAX);
+ uamp -= MAX14577_CHARGER_EOC_CURRENT_LIMIT_MIN;
+ current_bits = uamp / MAX14577_CHARGER_EOC_CURRENT_LIMIT_STEP;
+ break;
+ }
+
+ reg_data = current_bits << CHGCTRL5_EOCS_SHIFT;
+
+ return max14577_update_reg(chg->max14577->regmap,
+ MAX14577_CHG_REG_CHG_CTRL5, CHGCTRL5_EOCS_MASK,
+ reg_data);
+}
+
+static inline int max14577_init_fast_charge(struct max14577_charger *chg,
+ unsigned int uamp)
+{
+ u8 reg_data;
+ int ret;
+ const struct maxim_charger_current *limits =
+ &maxim_charger_currents[chg->max14577->dev_type];
+
+ ret = maxim_charger_calc_reg_current(limits, uamp, uamp, ®_data);
+ if (ret) {
+ dev_err(chg->dev, "Wrong value for fast charge: %u\n", uamp);
+ return ret;
+ }
+
+ return max14577_update_reg(chg->max14577->regmap,
+ MAX14577_CHG_REG_CHG_CTRL4,
+ CHGCTRL4_MBCICHWRCL_MASK | CHGCTRL4_MBCICHWRCH_MASK,
+ reg_data);
+}
+
/*
* Sets charger registers to proper and safe default values.
* Some of these values are equal to defaults in MAX14577E
* data sheet but there are minor differences.
*/
-static void max14577_charger_reg_init(struct max14577_charger *chg)
+static int max14577_charger_reg_init(struct max14577_charger *chg)
{
struct regmap *rmap = chg->max14577->regmap;
u8 reg_data;
+ int ret;
/*
* Charger-Type Manual Detection, default off (set CHGTYPMAN to 0)
@@ -198,10 +317,6 @@ static void max14577_charger_reg_init(struct max14577_charger *chg)
CDETCTRL1_CHGDETEN_MASK | CDETCTRL1_CHGTYPMAN_MASK,
reg_data);
- /* Battery Fast-Charge Timer, set to: 6hrs */
- reg_data = 0x3 << CHGCTRL1_TCHW_SHIFT;
- max14577_write_reg(rmap, MAX14577_REG_CHGCTRL1, reg_data);
-
/*
* Wall-Adapter Rapid Charge, default on
* Battery-Charger, default on
@@ -210,32 +325,46 @@ static void max14577_charger_reg_init(struct max14577_charger *chg)
reg_data |= 0x1 << CHGCTRL2_MBCHOSTEN_SHIFT;
max14577_write_reg(rmap, MAX14577_REG_CHGCTRL2, reg_data);
- /* Battery-Charger Constant Voltage (CV) Mode, set to: 4.35V */
- reg_data = 0xf << CHGCTRL3_MBCCVWRC_SHIFT;
- max14577_write_reg(rmap, MAX14577_REG_CHGCTRL3, reg_data);
-
- /*
- * Fast Battery-Charge Current Low,
- * default 200-950mA (max14577) / 100-475mA (max77836)
- *
- * Fast Battery-Charge Current High,
- * set to 450mA (max14577) / 225mA (max77836)
- */
- reg_data = 0x1 << CHGCTRL4_MBCICHWRCL_SHIFT;
- reg_data |= 0x5 << CHGCTRL4_MBCICHWRCH_SHIFT;
- max14577_write_reg(rmap, MAX14577_REG_CHGCTRL4, reg_data);
-
- /* End-of-Charge Current, set to 50mA (max14577) / 7.5mA (max77836) */
- reg_data = 0x0 << CHGCTRL5_EOCS_SHIFT;
- max14577_write_reg(rmap, MAX14577_REG_CHGCTRL5, reg_data);
-
/* Auto Charging Stop, default off */
reg_data = 0x0 << CHGCTRL6_AUTOSTOP_SHIFT;
max14577_write_reg(rmap, MAX14577_REG_CHGCTRL6, reg_data);
- /* Overvoltage-Protection Threshold, set to 6.5V */
- reg_data = 0x2 << CHGCTRL7_OTPCGHCVS_SHIFT;
+ ret = max14577_init_constant_voltage(chg, chg->pdata->constant_uvolt);
+ if (ret)
+ return ret;
+
+ ret = max14577_init_eoc(chg, chg->pdata->eoc_uamp);
+ if (ret)
+ return ret;
+
+ ret = max14577_init_fast_charge(chg, chg->pdata->fast_charge_uamp);
+ if (ret)
+ return ret;
+
+ ret = max14577_set_fast_charge_timer(chg,
+ MAXIM_CHARGER_FAST_CHARGE_TIMER_DEFAULT);
+ if (ret)
+ return ret;
+
+ /* Initialize Overvoltage-Protection Threshold */
+ switch (chg->pdata->ovp_uvolt) {
+ case 7500000:
+ reg_data = 0x0;
+ break;
+ case 6000000:
+ case 6500000:
+ case 7000000:
+ reg_data = 0x1 + (chg->pdata->ovp_uvolt - 6000000) / 500000;
+ break;
+ default:
+ dev_err(chg->dev, "Wrong value for OVP: %u\n",
+ chg->pdata->ovp_uvolt);
+ return -EINVAL;
+ }
+ reg_data <<= CHGCTRL7_OTPCGHCVS_SHIFT;
max14577_write_reg(rmap, MAX14577_REG_CHGCTRL7, reg_data);
+
+ return 0;
}
/* Support property from charger */
@@ -295,6 +424,110 @@ static int max14577_charger_get_property(struct power_supply *psy,
return ret;
}
+#ifdef CONFIG_OF
+static struct max14577_charger_platform_data *max14577_charger_dt_init(
+ struct platform_device *pdev)
+{
+ struct max14577_charger_platform_data *pdata;
+ struct device_node *np = pdev->dev.of_node;
+ int ret;
+
+ if (!np) {
+ dev_err(&pdev->dev, "No charger OF node\n");
+ return ERR_PTR(-EINVAL);
+ }
+
+ pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
+ if (!pdata)
+ return ERR_PTR(-ENOMEM);
+
+ ret = of_property_read_u32(np, "maxim,constant-uvolt",
+ &pdata->constant_uvolt);
+ if (ret) {
+ dev_err(&pdev->dev, "Cannot parse maxim,constant-uvolt field from DT\n");
+ return ERR_PTR(ret);
+ }
+
+ ret = of_property_read_u32(np, "maxim,fast-charge-uamp",
+ &pdata->fast_charge_uamp);
+ if (ret) {
+ dev_err(&pdev->dev, "Cannot parse maxim,fast-charge-uamp field from DT\n");
+ return ERR_PTR(ret);
+ }
+
+ ret = of_property_read_u32(np, "maxim,eoc-uamp", &pdata->eoc_uamp);
+ if (ret) {
+ dev_err(&pdev->dev, "Cannot parse maxim,eoc-uamp field from DT\n");
+ return ERR_PTR(ret);
+ }
+
+ ret = of_property_read_u32(np, "maxim,ovp-uvolt", &pdata->ovp_uvolt);
+ if (ret) {
+ dev_err(&pdev->dev, "Cannot parse maxim,ovp-uvolt field from DT\n");
+ return ERR_PTR(ret);
+ }
+
+ return pdata;
+}
+#else /* CONFIG_OF */
+static struct max14577_charger_platform_data *max14577_charger_dt_init(
+ struct platform_device *pdev)
+{
+ return NULL;
+}
+#endif /* CONFIG_OF */
+
+static ssize_t show_fast_charge_timer(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct max14577_charger *chg = dev_get_drvdata(dev);
+ u8 reg_data;
+ int ret;
+ unsigned int val;
+
+ ret = max14577_read_reg(chg->max14577->regmap, MAX14577_REG_CHGCTRL1,
+ ®_data);
+ if (ret)
+ return ret;
+
+ reg_data &= CHGCTRL1_TCHW_MASK;
+ reg_data >>= CHGCTRL1_TCHW_SHIFT;
+ switch (reg_data) {
+ case 0x2 ... 0x4:
+ val = reg_data + 3;
+ break;
+ case 0x7:
+ val = 0;
+ break;
+ default:
+ val = 5;
+ break;
+ }
+
+ return scnprintf(buf, PAGE_SIZE, "%u\n", val);
+}
+
+static ssize_t store_fast_charge_timer(struct device *dev,
+ struct device_attribute *attr, const char *buf, size_t count)
+{
+ struct max14577_charger *chg = dev_get_drvdata(dev);
+ unsigned long val;
+ int ret;
+
+ ret = kstrtoul(buf, 10, &val);
+ if (ret)
+ return ret;
+
+ ret = max14577_set_fast_charge_timer(chg, val);
+ if (ret)
+ return ret;
+
+ return count;
+}
+
+static DEVICE_ATTR(fast_charge_timer, S_IRUGO | S_IWUSR,
+ show_fast_charge_timer, store_fast_charge_timer);
+
static int max14577_charger_probe(struct platform_device *pdev)
{
struct max14577_charger *chg;
@@ -309,7 +542,13 @@ static int max14577_charger_probe(struct platform_device *pdev)
chg->dev = &pdev->dev;
chg->max14577 = max14577;
- max14577_charger_reg_init(chg);
+ chg->pdata = max14577_charger_dt_init(pdev);
+ if (IS_ERR_OR_NULL(chg->pdata))
+ return PTR_ERR(chg->pdata);
+
+ ret = max14577_charger_reg_init(chg);
+ if (ret)
+ return ret;
chg->charger.name = "max14577-charger",
chg->charger.type = POWER_SUPPLY_TYPE_BATTERY,
@@ -317,19 +556,35 @@ static int max14577_charger_probe(struct platform_device *pdev)
chg->charger.num_properties = ARRAY_SIZE(max14577_charger_props),
chg->charger.get_property = max14577_charger_get_property,
+ ret = device_create_file(&pdev->dev, &dev_attr_fast_charge_timer);
+ if (ret) {
+ dev_err(&pdev->dev, "failed: create sysfs entry\n");
+ return ret;
+ }
+
ret = power_supply_register(&pdev->dev, &chg->charger);
if (ret) {
dev_err(&pdev->dev, "failed: power supply register\n");
- return ret;
+ goto err;
}
+ /* Check for valid values for charger */
+ BUILD_BUG_ON(MAX14577_CHARGER_EOC_CURRENT_LIMIT_MIN +
+ MAX14577_CHARGER_EOC_CURRENT_LIMIT_STEP * 0xf !=
+ MAX14577_CHARGER_EOC_CURRENT_LIMIT_MAX);
return 0;
+
+err:
+ device_remove_file(&pdev->dev, &dev_attr_fast_charge_timer);
+
+ return ret;
}
static int max14577_charger_remove(struct platform_device *pdev)
{
struct max14577_charger *chg = platform_get_drvdata(pdev);
+ device_remove_file(&pdev->dev, &dev_attr_fast_charge_timer);
power_supply_unregister(&chg->charger);
return 0;
diff --git a/include/linux/mfd/max14577-private.h b/include/linux/mfd/max14577-private.h
index 7d514839c764..f01c1fae4d84 100644
--- a/include/linux/mfd/max14577-private.h
+++ b/include/linux/mfd/max14577-private.h
@@ -293,6 +293,25 @@ enum max14577_charger_reg {
#define MAX77836_CHARGER_CURRENT_LIMIT_HIGH_STEP 25000U
#define MAX77836_CHARGER_CURRENT_LIMIT_MAX 475000U
+/*
+ * MAX14577 charger End-Of-Charge current limits
+ * (as in CHGCTRL5 register), uA
+ */
+#define MAX14577_CHARGER_EOC_CURRENT_LIMIT_MIN 50000U
+#define MAX14577_CHARGER_EOC_CURRENT_LIMIT_STEP 10000U
+#define MAX14577_CHARGER_EOC_CURRENT_LIMIT_MAX 200000U
+
+/*
+ * MAX14577/MAX77836 Battery Constant Voltage
+ * (as in CHGCTRL3 register), uV
+ */
+#define MAXIM_CHARGER_CONSTANT_VOLTAGE_MIN 4000000U
+#define MAXIM_CHARGER_CONSTANT_VOLTAGE_STEP 20000U
+#define MAXIM_CHARGER_CONSTANT_VOLTAGE_MAX 4350000U
+
+/* Default value for fast charge timer, in hours */
+#define MAXIM_CHARGER_FAST_CHARGE_TIMER_DEFAULT 5
+
/* MAX14577 regulator SFOUT LDO voltage, fixed, uV */
#define MAX14577_REGULATOR_SAFEOUT_VOLTAGE 4900000
diff --git a/include/linux/mfd/max14577.h b/include/linux/mfd/max14577.h
index 3c098d57b1d1..ccfaf952c31b 100644
--- a/include/linux/mfd/max14577.h
+++ b/include/linux/mfd/max14577.h
@@ -54,6 +54,13 @@ struct max14577_regulator_platform_data {
struct device_node *of_node;
};
+struct max14577_charger_platform_data {
+ u32 constant_uvolt;
+ u32 fast_charge_uamp;
+ u32 eoc_uamp;
+ u32 ovp_uvolt;
+};
+
/*
* MAX14577 MFD platform data
*/
--
1.9.1
^ permalink raw reply related
* [PATCH v4 6/8] power: max17040: Add ID for MAX77836 Fuel Gauge block
From: Krzysztof Kozlowski @ 2014-08-27 15:20 UTC (permalink / raw)
To: Sebastian Reichel, Dmitry Eremin-Solenikov, David Woodhouse,
Samuel Ortiz, Lee Jones, linux-kernel, linux-api, linux-pm
Cc: Liam Girdwood, Mark Brown, Rob Herring, Pawel Moll, Mark Rutland,
Ian Campbell, Kumar Gala, Kyungmin Park, Marek Szyprowski,
Bartlomiej Zolnierkiewicz, Krzysztof Kozlowski
In-Reply-To: <1409152814-17302-1-git-send-email-k.kozlowski@samsung.com>
MAX77836 has the same Fuel Gauge as MAX17040/17048. The max17040 driver
can be safely re-used. The patch adds MAX77836 ID to array of
i2c_device_id.
Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Cc: Kyungmin Park <kyungmin.park@samsung.com>
---
drivers/power/max17040_battery.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/power/max17040_battery.c b/drivers/power/max17040_battery.c
index 0fbac861080d..165ffe381803 100644
--- a/drivers/power/max17040_battery.c
+++ b/drivers/power/max17040_battery.c
@@ -278,6 +278,7 @@ static SIMPLE_DEV_PM_OPS(max17040_pm_ops, max17040_suspend, max17040_resume);
static const struct i2c_device_id max17040_id[] = {
{ "max17040", 0 },
+ { "max77836-battery", 0 },
{ }
};
MODULE_DEVICE_TABLE(i2c, max17040_id);
--
1.9.1
^ permalink raw reply related
* [PATCH v4 7/8] devicetree: mfd: max14577: Add device tree bindings document
From: Krzysztof Kozlowski @ 2014-08-27 15:20 UTC (permalink / raw)
To: Sebastian Reichel, Dmitry Eremin-Solenikov, David Woodhouse,
Samuel Ortiz, Lee Jones, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA, linux-pm-u79uwXL29TY76Z2rM5mHXA
Cc: Liam Girdwood, Mark Brown, Rob Herring, Pawel Moll, Mark Rutland,
Ian Campbell, Kumar Gala, Kyungmin Park, Marek Szyprowski,
Bartlomiej Zolnierkiewicz, Krzysztof Kozlowski,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1409152814-17302-1-git-send-email-k.kozlowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
Add document describing device tree bindings for MAX14577 MFD
drivers: MFD core, extcon, regulator and charger.
Both MAX14577 and MAX77836 chipsets are documented.
Signed-off-by: Krzysztof Kozlowski <k.kozlowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
Cc: Kyungmin Park <kyungmin.park-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Reviewed-by: Tomasz Figa <t.figa-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
---
Documentation/devicetree/bindings/mfd/max14577.txt | 146 +++++++++++++++++++++
1 file changed, 146 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mfd/max14577.txt
diff --git a/Documentation/devicetree/bindings/mfd/max14577.txt b/Documentation/devicetree/bindings/mfd/max14577.txt
new file mode 100644
index 000000000000..236264c10b92
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/max14577.txt
@@ -0,0 +1,146 @@
+Maxim MAX14577/77836 Multi-Function Device
+
+MAX14577 is a Multi-Function Device with Micro-USB Interface Circuit, Li+
+Battery Charger and SFOUT LDO output for powering USB devices. It is
+interfaced to host controller using I2C.
+
+MAX77836 additionally contains PMIC (with two LDO regulators) and Fuel Gauge.
+
+
+Required properties:
+- compatible : Must be "maxim,max14577" or "maxim,max77836".
+- reg : I2C slave address for the max14577 chip (0x25 for max14577/max77836)
+- interrupts : IRQ line for the chip.
+- interrupt-parent : The parent interrupt controller.
+
+
+Required nodes:
+ - charger :
+ Node for configuring the charger driver.
+ Required properties:
+ - compatible : "maxim,max14577-charger"
+ or "maxim,max77836-charger"
+ - maxim,fast-charge-uamp : Current in uA for Fast Charge;
+ Valid values:
+ - for max14577: 90000 - 950000;
+ - for max77836: 45000 - 475000;
+ - maxim,eoc-uamp : Current in uA for End-Of-Charge mode;
+ Valid values:
+ - for max14577: 50000 - 200000;
+ - for max77836: 5000 - 100000;
+ - maxim,ovp-uvolt : OverVoltage Protection Threshold in uV;
+ In an overvoltage condition, INT asserts and charging
+ stops. Valid values:
+ - 6000000, 6500000, 7000000, 7500000;
+ - maxim,constant-uvolt : Battery Constant Voltage in uV;
+ Valid values:
+ - 4000000 - 4280000 (step by 20000);
+ - 4350000;
+
+
+Optional nodes:
+- max14577-muic/max77836-muic :
+ Node used only by extcon consumers.
+ Required properties:
+ - compatible : "maxim,max14577-muic" or "maxim,max77836-muic"
+
+- regulators :
+ Required properties:
+ - compatible : "maxim,max14577-regulator"
+ or "maxim,max77836-regulator"
+
+ May contain a sub-node per regulator from the list below. Each
+ sub-node should contain the constraints and initialization information
+ for that regulator. See regulator.txt for a description of standard
+ properties for these sub-nodes.
+
+ List of valid regulator names:
+ - for max14577: CHARGER, SAFEOUT.
+ - for max77836: CHARGER, SAFEOUT, LDO1, LDO2.
+
+ The SAFEOUT is a fixed voltage regulator so there is no need to specify
+ voltages for it.
+
+
+Example:
+
+#include <dt-bindings/interrupt-controller/irq.h>
+
+max14577@25 {
+ compatible = "maxim,max14577";
+ reg = <0x25>;
+ interrupt-parent = <&gpx1>;
+ interrupts = <5 IRQ_TYPE_NONE>;
+
+ muic: max14577-muic {
+ compatible = "maxim,max14577-muic";
+ };
+
+ regulators {
+ compatible = "maxim,max14577-regulator";
+
+ SAFEOUT {
+ regulator-name = "SAFEOUT";
+ };
+ CHARGER {
+ regulator-name = "CHARGER";
+ regulator-min-microamp = <90000>;
+ regulator-max-microamp = <950000>;
+ regulator-boot-on;
+ };
+ };
+
+ charger {
+ compatible = "maxim,max14577-charger";
+
+ maxim,constant-uvolt = <4350000>;
+ maxim,fast-charge-uamp = <450000>;
+ maxim,eoc-uamp = <50000>;
+ maxim,ovp-uvolt = <6500000>;
+ };
+};
+
+
+max77836@25 {
+ compatible = "maxim,max77836";
+ reg = <0x25>;
+ interrupt-parent = <&gpx1>;
+ interrupts = <5 IRQ_TYPE_NONE>;
+
+ muic: max77836-muic {
+ compatible = "maxim,max77836-muic";
+ };
+
+ regulators {
+ compatible = "maxim,max77836-regulator";
+
+ SAFEOUT {
+ regulator-name = "SAFEOUT";
+ };
+ CHARGER {
+ regulator-name = "CHARGER";
+ regulator-min-microamp = <90000>;
+ regulator-max-microamp = <950000>;
+ regulator-boot-on;
+ };
+ LDO1 {
+ regulator-name = "LDO1";
+ regulator-min-microvolt = <2700000>;
+ regulator-max-microvolt = <2700000>;
+ };
+ LDO2 {
+ regulator-name = "LDO2";
+ regulator-min-microvolt = <800000>;
+ regulator-max-microvolt = <3950000>;
+ };
+ };
+
+ charger {
+ compatible = "maxim,max77836-charger";
+
+ maxim,constant-uvolt = <4350000>;
+ maxim,fast-charge-uamp = <225000>;
+ maxim,eoc-uamp = <7500>;
+ maxim,ovp-uvolt = <6500000>;
+ };
+};
--
1.9.1
^ permalink raw reply related
* [PATCH v4 8/8] Documentation: charger: max14577: Document exported sysfs entry
From: Krzysztof Kozlowski @ 2014-08-27 15:20 UTC (permalink / raw)
To: Sebastian Reichel, Dmitry Eremin-Solenikov, David Woodhouse,
Samuel Ortiz, Lee Jones, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA, linux-pm-u79uwXL29TY76Z2rM5mHXA
Cc: Liam Girdwood, Mark Brown, Rob Herring, Pawel Moll, Mark Rutland,
Ian Campbell, Kumar Gala, Kyungmin Park, Marek Szyprowski,
Bartlomiej Zolnierkiewicz, Krzysztof Kozlowski
In-Reply-To: <1409152814-17302-1-git-send-email-k.kozlowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
Document the 'fast charge timer' setting exported by max14577 driver
through sysfs entry.
Signed-off-by: Krzysztof Kozlowski <k.kozlowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
Cc: Kyungmin Park <kyungmin.park-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
---
Documentation/ABI/testing/sysfs-class-power | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/Documentation/ABI/testing/sysfs-class-power b/Documentation/ABI/testing/sysfs-class-power
index 78c7baca3587..83ee67ebf0e9 100644
--- a/Documentation/ABI/testing/sysfs-class-power
+++ b/Documentation/ABI/testing/sysfs-class-power
@@ -18,3 +18,17 @@ Description:
This file is writeable and can be used to set the assumed
battery 'full level'. As batteries age, this value has to be
amended over time.
+
+What: /sys/class/power_supply/max14577-charger/device/fast_charge_timer
+Date: July 2014
+KernelVersion: 3.17.0
+Contact: Krzysztof Kozlowski <k.kozlowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
+Description:
+ This entry shows and sets the maximum time the max14577
+ charger operates in fast-charge mode. When the timer expires
+ the device will terminate fast-charge mode (charging current
+ will drop to 0 A) and will trigger interrupt.
+
+ Valid values:
+ - 5, 6 or 7 (hours),
+ - 0: disabled.
--
1.9.1
^ permalink raw reply related
* Re: [RESEND PATCH v3 1/6] charger: max14577: Add support for MAX77836 charger
From: Lee Jones @ 2014-08-27 17:04 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Sebastian Reichel, Dmitry Eremin-Solenikov, David Woodhouse,
linux-kernel, linux-api, linux-pm, Samuel Ortiz, Liam Girdwood,
Mark Brown, Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell,
Kumar Gala, Kyungmin Park, Marek Szyprowski,
Bartlomiej Zolnierkiewicz, Anton Vorontsov
In-Reply-To: <1409148089.25748.5.camel@AMDC1943>
On Wed, 27 Aug 2014, Krzysztof Kozlowski wrote:
> On śro, 2014-08-27 at 14:52 +0100, Lee Jones wrote:
> > On Wed, 27 Aug 2014, Krzysztof Kozlowski wrote:
> >
> > > Add support for MAX77836 charger to the max14577 driver. The MAX77836
> > > charger is almost the same as 14577 model except:
> > > - No dead-battery detection;
> > > - Support for special charger (like in MAX77693);
> > > - Support for DX over-voltage protection (like in MAX77693);
> > > - Lower values of charging current (two times lower current for
> > > slow/fast charge, much lower EOC current);
> > > - Slightly different values in ChgTyp field of STATUS2 register. On
> > > MAX14577 0x6 is reserved and 0x7 dead battery. On the MAX77836 the
> > > 0x6 means special charger and 0x7 is reserved. Regardless of these
> > > differences the driver maps them to one enum max14577_muic_charger_type.
> > >
> > > Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> > > Cc: Kyungmin Park <kyungmin.park@samsung.com>
> > > Cc: Anton Vorontsov <anton@enomsg.org>
> > > Cc: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
> > > Cc: David Woodhouse <dwmw2@infradead.org>
> > > Acked-by: Lee Jones <lee.jones@linaro.org>
> > > ---
> > > drivers/power/Kconfig | 4 +-
> > > drivers/power/max14577_charger.c | 77 +++++++++++++++++++++++++++++-------
> > > include/linux/mfd/max14577-private.h | 54 ++++++++++++++++++-------
> >
> > Not quite so important here, but better to break these out too. We
> > can arrange ourselves to that the set can go in together. Just make
> > sure it's bisectable.
>
> Are you sure? This will result in a patch which adds some
> code/symbols/defines used only by next (future) patch. Independent apply
> of such patch still won't make any sense because there won't be any user
> of this code.
That's fine. I take in lots of patches which do nothing more than
adds/updates infrastructure.
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply
* Re: [RESEND PATCH v3 3/6] charger: max14577: Configure battery-dependent settings from DTS and sysfs
From: Lee Jones @ 2014-08-27 17:05 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Sebastian Reichel, Dmitry Eremin-Solenikov, David Woodhouse,
linux-kernel, linux-api, linux-pm, Samuel Ortiz, Liam Girdwood,
Mark Brown, Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell,
Kumar Gala, Kyungmin Park, Marek Szyprowski,
Bartlomiej Zolnierkiewicz, Jenny Tc
In-Reply-To: <1409147725.25748.1.camel@AMDC1943>
On Wed, 27 Aug 2014, Krzysztof Kozlowski wrote:
> On śro, 2014-08-27 at 14:50 +0100, Lee Jones wrote:
> > On Wed, 27 Aug 2014, Krzysztof Kozlowski wrote:
> >
> > > Remove hard-coded values for:
> > > - Fast Charge current,
> > > - End Of Charge current,
> > > - Fast Charge timer,
> > > - Overvoltage Protection Threshold,
> > > - Battery Constant Voltage,
> > > and use DTS or sysfs to configure them. This allows using the max14577 charger
> > > driver with different batteries.
> > >
> > > Now the charger driver requires valid configuration data from DTS. In
> > > case of wrong configuration data it fails during probe. Patch adds
> > > of_compatible to the charger mfd cell in MFD driver core.
> > >
> > > The fast charge timer is configured through sysfs entry.
> > >
> > > Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> > > Cc: Kyungmin Park <kyungmin.park@samsung.com>
> > > Cc: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
> > > Cc: David Woodhouse <dwmw2@infradead.org>
> > > Cc: Jenny Tc <jenny.tc@intel.com>
> > > Cc: Mark Rutland <mark.rutland@arm.com>
> > > Acked-by: Lee Jones <lee.jones@linaro.org>
> > > ---
> > > drivers/mfd/max14577.c | 5 +-
> >
> > Hmm... not sure why I Acked this. The code looks okay, but can you
> > break out the MFD changed into a separate patch please? You can add
> > my Ack to the broken out patch though.
>
> Sure, I'll split it however the compatible here in mfd_cell makes sense
> only with this patch.
That's completely fine.
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply
* Re: [PATCH RFC v7 net-next 00/28] BPF syscall
From: Steven Stewart-Gallus @ 2014-08-27 18:24 UTC (permalink / raw)
To: David Miller
Cc: mingo-DgEjT+Ai2ygdnm+yROfE0A,
torvalds-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
luto-kltTT9wpgjJwATOyAt5JVQ, rostedt-nx8X9YLhiw1AfugRpC6u6w,
dborkman-H+wXaHxf7aLQT0dZR+AlfA, chema-hpIqsD4AKlfQT0dZR+AlfA,
edumazet-hpIqsD4AKlfQT0dZR+AlfA,
a.p.zijlstra-/NLkJaSkS4VmR6Xm/wNWPw,
brendan.d.gregg-Re5JQEeQqe8AvxtiuMwx3w,
namhyung-DgEjT+Ai2ygdnm+yROfE0A, hpa-YMNOUZJC4hwAvxtiuMwx3w,
akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
keescook-F7+t8E8rja9g9hUCZPvPmw, linux-api-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20140826.231155.421325307812864648.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
> I'm personally not reviewing such a large patch series, sorry.
>
> You need to submit smaller sets if you want to get reasonable
> review of your changes and ideas.
Hello. As well, this clogs up the mailing boxes of other people who
have no interest in the patch set.
Thank you,
Steven Stewart-Gallus
^ permalink raw reply
* Re: [PATCH RFC v7 net-next 00/28] BPF syscall
From: Andy Lutomirski @ 2014-08-27 18:26 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Daniel Borkmann, H. Peter Anvin, Andrew Morton, Chema Gonzalez,
Namhyung Kim, Eric Dumazet, David S. Miller,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Brendan Gregg, Linus Torvalds, Steven Rostedt,
Network Development, Peter Zijlstra, Kees Cook, Linux API,
Ingo Molnar
In-Reply-To: <CAMEtUuw1n1HzAeyKFj9=nGq7RKZq7TADS-6M_BkHbTsWJ_Gm-Q-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Tue, Aug 26, 2014 at 9:57 PM, Alexei Starovoitov <ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org> wrote:
> On Tue, Aug 26, 2014 at 9:49 PM, Andy Lutomirski <luto-kltTT9wpgjJwATOyAt5JVQ@public.gmane.org> wrote:
>> On Tue, Aug 26, 2014 at 9:35 PM, Alexei Starovoitov <ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org> wrote:
>>> On Tue, Aug 26, 2014 at 8:56 PM, Andy Lutomirski <luto-kltTT9wpgjJwATOyAt5JVQ@public.gmane.org> wrote:
>>>> On Aug 26, 2014 7:29 PM, "Alexei Starovoitov" <ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org> wrote:
>>>>>
>>>>> Hi Ingo, David,
>>>>>
>>>>> posting whole thing again as RFC to get feedback on syscall only.
>>>>> If syscall bpf(int cmd, union bpf_attr *attr, unsigned int size) is ok,
>>>>> I'll split them into small chunks as requested and will repost without RFC.
>>>>
>>>> IMO it's much easier to review a syscall if we just look at a
>>>> specification of what it does. The code is, in some sense, secondary.
>>>
>>> 'specification of what it does'... hmm, you mean beyond what's
>>> there in commit logs and in Documentation/networking/filter.txt ?
>>> Aren't samples at the end give an idea on 'what it does'?
>>> I'm happy to add 'specification', I just don't understand yet what
>>> it suppose to talk about beyond what's already written.
>>> I understand that the patches are missing explanation on 'why'
>>> the syscall is being added, but I don't think it's what you're asking...
>>
>> I mean a hopefully short document that defines what the syscall does.
>> It should be precise enough that one could, in principle, implement
>> the syscall just by reading the document and that one could use the
>> syscall just by reading the document.
>>
>> Given that there's a whole instruction set to go with it, it may end
>> up being moderately complicated or saying things like "see this other
>> thing for a description of the instruction set" and "there are some
>> extensible sets of functions you can call with it".
>
> I'm still lost.
>
> Here is the quote from Documentation/networking/filter.txt
> "
> 'maps' is a generic storage of different types for sharing data between kernel
> and userspace.
>
> The maps are accessed from user space via BPF syscall,
> which has commands:
> - create a map with given type and attributes
> map_fd = bpf(BPF_MAP_CREATE, union bpf_attr *attr, u32 size)
> using attr->map_type, attr->key_size, attr->value_size, attr->max_entries
> returns process-local file descriptor or negative error
>
> - lookup key in a given map
> err = bpf(BPF_MAP_LOOKUP_ELEM, union bpf_attr *attr, u32 size)
> using attr->map_fd, attr->key, attr->value
> returns zero and stores found elem into value or negative error
>
> - create or update key/value pair in a given map
> err = bpf(BPF_MAP_UPDATE_ELEM, union bpf_attr *attr, u32 size)
> using attr->map_fd, attr->key, attr->value
> returns zero or negative error
>
> - find and delete element by key in a given map
> err = bpf(BPF_MAP_DELETE_ELEM, union bpf_attr *attr, u32 size)
> using attr->map_fd, attr->key
>
> - to delete map: close(fd)
> Exiting process will delete maps automatically
>
> userspace programs uses this API to create/populate/read
> maps that eBPF programs are concurrently updating.
> "
> and more in commit log:
> "
> - load eBPF program
> fd = bpf(BPF_PROG_LOAD, union bpf_attr *attr, u32 size)
>
> where 'attr' is
> struct {
> enum bpf_prog_type prog_type;
> __u32 insn_cnt;
> struct bpf_insn __user *insns;
> const char __user *license;
> };
> insns - array of eBPF instructions
> license - must be GPL compatible to call helper functions marked gpl_only
>
> - unload eBPF program
> close(fd)
> "
>
> Isn't it short and describes what it does?
> Do you want me to describe what eBPF program can do?
The problem is that everyone needs to dig around a very long patch
series to find it. Since you're asking for a review of a syscall, it
would be nice to have everything needed to review whether the syscall
is a good idea in its present form in one place and to keep the amount
of email under control.
--Andy
--
Andy Lutomirski
AMA Capital Management, LLC
^ permalink raw reply
* Re: [PATCH RFC v7 net-next 00/28] BPF syscall
From: Stephen Hemminger @ 2014-08-27 19:18 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Alexei Starovoitov, Daniel Borkmann, H. Peter Anvin,
Andrew Morton, Chema Gonzalez, Namhyung Kim, Eric Dumazet,
David S. Miller,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Brendan Gregg, Linus Torvalds, Steven Rostedt,
Network Development, Peter Zijlstra, Kees Cook, Linux API,
Ingo Molnar
In-Reply-To: <CALCETrXAfZJTsF2nPFw55rHkfbNXKQuF8Frnq3e1wHEoGxLM4w-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
Something in man page format similar to FreeBSD man page:
http://www.freebsd.org/cgi/man.cgi?bpf(4)
would be more readable and reviewable.
^ permalink raw reply
* Re: [PATCH RFC v7 net-next 00/28] BPF syscall
From: Daniel Borkmann @ 2014-08-27 19:35 UTC (permalink / raw)
To: Stephen Hemminger
Cc: Andy Lutomirski, Alexei Starovoitov, H. Peter Anvin,
Andrew Morton, Chema Gonzalez, Namhyung Kim, Eric Dumazet,
David S. Miller,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Brendan Gregg, Linus Torvalds, Steven Rostedt,
Network Development, Peter Zijlstra, Kees Cook, Linux API,
Ingo Molnar
In-Reply-To: <20140827121801.3bfd916e@urahara>
On 08/27/2014 09:18 PM, Stephen Hemminger wrote:
> Something in man page format similar to FreeBSD man page:
> http://www.freebsd.org/cgi/man.cgi?bpf(4)
>
> would be more readable and reviewable.
I think at some point, we could perhaps do a section 7 page
with a general overview of the engine and where it can be
applied, and let the syscall page partially refer to it so
that it doesn't get too long. So far, we tried to squeeze
everything into Documentation/networking/filter.txt, and that
itself is quite long already.
^ permalink raw reply
* Re: [PATCH RFC v7 net-next 00/28] BPF syscall
From: Alexei Starovoitov @ 2014-08-27 19:37 UTC (permalink / raw)
To: Stephen Hemminger
Cc: Andy Lutomirski, Daniel Borkmann, H. Peter Anvin, Andrew Morton,
Chema Gonzalez, Namhyung Kim, Eric Dumazet, David S. Miller,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Brendan Gregg, Linus Torvalds, Steven Rostedt,
Network Development, Peter Zijlstra, Kees Cook, Linux API,
Ingo Molnar
In-Reply-To: <20140827121801.3bfd916e@urahara>
On Wed, Aug 27, 2014 at 12:18 PM, Stephen Hemminger
<stephen-OTpzqLSitTUnbdJkjeBofR2eb7JE58TQ@public.gmane.org> wrote:
> Something in man page format similar to FreeBSD man page:
> http://www.freebsd.org/cgi/man.cgi?bpf(4)
>
> would be more readable and reviewable.
Ok. will chop it into smallest diff possible and will add a doc
for syscall only. I guess the problem is that we have too many
docs now that talk about everything.
^ permalink raw reply
* [PATCH v8 net-next 0/2] load imm64 insn and uapi/linux/bpf.h
From: Alexei Starovoitov @ 2014-08-27 20:37 UTC (permalink / raw)
To: David S. Miller
Cc: Ingo Molnar, Linus Torvalds, Andy Lutomirski, Steven Rostedt,
Daniel Borkmann, Chema Gonzalez, Eric Dumazet, Peter Zijlstra,
Brendan Gregg, Namhyung Kim, H. Peter Anvin, Andrew Morton,
Kees Cook, linux-api, netdev, linux-kernel
Hi David,
I've been thinking on the minimum first patch set.
Came up with the following two:
1st patch adds 'load 64-bit immediate' instruction which by itself
is harmless and used to load constants only. In the future we may
add pseudo variants of this insn, so user space can request
internal kernel pointer. More detailed explanation in the commit log.
2nd patch exposed eBPF ISA to user space. It moves 55 lines from
filter.h into uapi/linux/bpf.h
Though there is no way currently to load eBPF programs from user
space, this patch shows the intent that eventually it will be possible.
The main goal here is to unblock LLVM upstreaming process.
Once these two are in, I can start posting LLVM RFCs to llvmdev list
and getting compiler bits in, so by the time bpf syscall and verifier
are in, we may have LLVM backend upstreamed as well.
LLVM wouldn't care what eBPF is used for, whether syscall is used
or some other mechanism. It just compiles C into eBPF ISA.
So these two patches are sufficient to start LLVM upstreaming.
All,
why do we need all of these?
Same reason why we're still using classic BPF and keep trying to extend it.
There are places in kernel where safe dynamic programs are mandatory.
network traffic capture needs in-kernel filtering,
seccomp needs safe mini programs to sandbox applications,
tracing needs them to filter events and so on.
Few LWN articles that explain things way better than my commit logs:
http://lwn.net/Articles/599755/
http://lwn.net/Articles/603983/
http://lwn.net/Articles/606089/
http://lwn.net/Articles/575531/
The first target for eBPF is to have dtrace equivalent that can be
used in _production_. Safety of programs is paramount.
Just like performance. eBPF programs in tracing should not affect
performance of production severs, so huge effort on optimizing last bit.
ebpf+tracing, ebpf+seccomp, ebpf+sockets are the most obvious use cases.
ebpf+ovs is the one we use in large kvm hypervisors.
40Gbps of traffic are going through these programs, so performance
and safety are vital. Performance implications ruling out run-time checks
in critical path, so verifier is large mainly because it needs to do
all the checks during static analysis.
I think next patch set will include syscall shell with minimal functionality,
syscall doc and simple test.
Alexei Starovoitov (2):
net: filter: add "load 64-bit immediate" eBPF instruction
net: filter: split filter.h and expose eBPF to user space
Documentation/networking/filter.txt | 8 +++-
arch/x86/net/bpf_jit_comp.c | 17 ++++++++
include/linux/filter.h | 74 +++++++++--------------------------
include/uapi/linux/Kbuild | 1 +
include/uapi/linux/bpf.h | 65 ++++++++++++++++++++++++++++++
kernel/bpf/core.c | 5 +++
lib/test_bpf.c | 21 ++++++++++
7 files changed, 135 insertions(+), 56 deletions(-)
create mode 100644 include/uapi/linux/bpf.h
--
1.7.9.5
^ permalink raw reply
* [PATCH v8 net-next 1/2] net: filter: add "load 64-bit immediate" eBPF instruction
From: Alexei Starovoitov @ 2014-08-27 20:37 UTC (permalink / raw)
To: David S. Miller
Cc: Ingo Molnar, Linus Torvalds, Andy Lutomirski, Steven Rostedt,
Daniel Borkmann, Chema Gonzalez, Eric Dumazet, Peter Zijlstra,
Brendan Gregg, Namhyung Kim, H. Peter Anvin, Andrew Morton,
Kees Cook, linux-api, netdev, linux-kernel
In-Reply-To: <1409171833-6979-1-git-send-email-ast@plumgrid.com>
add BPF_LD_IMM64 instruction to load 64-bit immediate value into a register.
All previous instructions were 8-byte. This is first 16-byte instruction.
Two consecutive 'struct bpf_insn' blocks are interpreted as single instruction:
insn[0].code = BPF_LD | BPF_DW | BPF_IMM
insn[0].dst_reg = destination register
insn[0].imm = lower 32-bit
insn[1].code = 0
insn[1].imm = upper 32-bit
All unused fields must be zero.
Classic BPF has similar instruction: BPF_LD | BPF_W | BPF_IMM
which loads 32-bit immediate value into a register.
x64 JITs it as single 'movabsq %rax, imm64'
arm64 may JIT as sequence of four 'movk x0, #imm16, lsl #shift' insn
Note that old eBPF programs are binary compatible with new interpreter.
It helps eBPF programs load 64-bit constant into a register with one
instruction instead of using two registers and 4 instructions:
BPF_MOV32_IMM(R1, imm32)
BPF_ALU64_IMM(BPF_LSH, R1, 32)
BPF_MOV32_IMM(R2, imm32)
BPF_ALU64_REG(BPF_OR, R1, R2)
User space generated programs will use this instruction to load constants only.
To tell kernel that user space needs a pointer the _pseudo_ variant of
this instruction may be added later, which will use extra bits of encoding
to indicate what type of pointer user space is asking kernel to provide.
For example 'off' or 'src_reg' fields can be used for such purpose.
src_reg = 1 could mean that user space is asking kernel to validate and
load in-kernel map pointer.
src_reg = 2 could mean that user space needs readonly data section pointer
src_reg = 3 could mean that user space needs a pointer to per-cpu local data
All such future pseudo instructions will not be carrying the actual pointer
as part of the instruction, but rather will be treated as a request to kernel
to provide one. The kernel will verify the request_for_a_pointer, then
will drop pseudo marking and will store actual internal pointer inside
the instruction, so the end result is the interpreter and JITs never
see pseudo BPF_LD_IMM64 insns and only operate on single generic BPF_LD_IMM64.
User space never operates on direct pointers and verifier can easily
recognize request_for_pointer_pseudo_insn vs other instructions.
Signed-off-by: Alexei Starovoitov <ast@plumgrid.com>
---
Documentation/networking/filter.txt | 8 +++++++-
arch/x86/net/bpf_jit_comp.c | 17 +++++++++++++++++
include/linux/filter.h | 18 ++++++++++++++++++
kernel/bpf/core.c | 5 +++++
lib/test_bpf.c | 21 +++++++++++++++++++++
5 files changed, 68 insertions(+), 1 deletion(-)
diff --git a/Documentation/networking/filter.txt b/Documentation/networking/filter.txt
index c48a9704bda8..81916ab5d96f 100644
--- a/Documentation/networking/filter.txt
+++ b/Documentation/networking/filter.txt
@@ -951,7 +951,7 @@ Size modifier is one of ...
Mode modifier is one of:
- BPF_IMM 0x00 /* classic BPF only, reserved in eBPF */
+ BPF_IMM 0x00 /* used for 32-bit mov in classic BPF and 64-bit in eBPF */
BPF_ABS 0x20
BPF_IND 0x40
BPF_MEM 0x60
@@ -995,6 +995,12 @@ BPF_XADD | BPF_DW | BPF_STX: lock xadd *(u64 *)(dst_reg + off16) += src_reg
Where size is one of: BPF_B or BPF_H or BPF_W or BPF_DW. Note that 1 and
2 byte atomic increments are not supported.
+eBPF has one 16-byte instruction: BPF_LD | BPF_DW | BPF_IMM which consists
+of two consecutive 'struct bpf_insn' 8-byte blocks and interpreted as single
+instruction that loads 64-bit immediate value into a dst_reg.
+Classic BPF has similar instruction: BPF_LD | BPF_W | BPF_IMM which loads
+32-bit immediate value into a register.
+
Testing
-------
diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
index b08a98c59530..98837147ee57 100644
--- a/arch/x86/net/bpf_jit_comp.c
+++ b/arch/x86/net/bpf_jit_comp.c
@@ -393,6 +393,23 @@ static int do_jit(struct bpf_prog *bpf_prog, int *addrs, u8 *image,
EMIT1_off32(add_1reg(0xB8, dst_reg), imm32);
break;
+ case BPF_LD | BPF_IMM | BPF_DW:
+ if (insn[1].code != 0 || insn[1].src_reg != 0 ||
+ insn[1].dst_reg != 0 || insn[1].off != 0) {
+ /* verifier must catch invalid insns */
+ pr_err("invalid BPF_LD_IMM64 insn\n");
+ return -EINVAL;
+ }
+
+ /* movabsq %rax, imm64 */
+ EMIT2(add_1mod(0x48, dst_reg), add_1reg(0xB8, dst_reg));
+ EMIT(insn[0].imm, 4);
+ EMIT(insn[1].imm, 4);
+
+ insn++;
+ i++;
+ break;
+
/* dst %= src, dst /= src, dst %= imm32, dst /= imm32 */
case BPF_ALU | BPF_MOD | BPF_X:
case BPF_ALU | BPF_DIV | BPF_X:
diff --git a/include/linux/filter.h b/include/linux/filter.h
index a5227ab8ccb1..f3262b598262 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -161,6 +161,24 @@ enum {
.off = 0, \
.imm = IMM })
+/* BPF_LD_IMM64 macro encodes single 'load 64-bit immediate' insn */
+#define BPF_LD_IMM64(DST, IMM) \
+ BPF_LD_IMM64_RAW(DST, 0, IMM)
+
+#define BPF_LD_IMM64_RAW(DST, SRC, IMM) \
+ ((struct bpf_insn) { \
+ .code = BPF_LD | BPF_DW | BPF_IMM, \
+ .dst_reg = DST, \
+ .src_reg = SRC, \
+ .off = 0, \
+ .imm = (__u32) (IMM) }), \
+ ((struct bpf_insn) { \
+ .code = 0, /* zero is reserved opcode */ \
+ .dst_reg = 0, \
+ .src_reg = 0, \
+ .off = 0, \
+ .imm = ((__u64) (IMM)) >> 32 })
+
/* Short form of mov based on type, BPF_X: dst_reg = src_reg, BPF_K: dst_reg = imm32 */
#define BPF_MOV64_RAW(TYPE, DST, SRC, IMM) \
diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
index 7f0dbcbb34af..0434c2170f2b 100644
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -180,6 +180,7 @@ static unsigned int __bpf_prog_run(void *ctx, const struct bpf_insn *insn)
[BPF_LD | BPF_IND | BPF_W] = &&LD_IND_W,
[BPF_LD | BPF_IND | BPF_H] = &&LD_IND_H,
[BPF_LD | BPF_IND | BPF_B] = &&LD_IND_B,
+ [BPF_LD | BPF_IMM | BPF_DW] = &&LD_IMM_DW,
};
void *ptr;
int off;
@@ -239,6 +240,10 @@ select_insn:
ALU64_MOV_K:
DST = IMM;
CONT;
+ LD_IMM_DW:
+ DST = (u64) (u32) insn[0].imm | ((u64) (u32) insn[1].imm) << 32;
+ insn++;
+ CONT;
ALU64_ARSH_X:
(*(s64 *) &DST) >>= SRC;
CONT;
diff --git a/lib/test_bpf.c b/lib/test_bpf.c
index 8c66c6aace04..46ab1a7ef135 100644
--- a/lib/test_bpf.c
+++ b/lib/test_bpf.c
@@ -1735,6 +1735,27 @@ static struct bpf_test tests[] = {
{ },
{ { 1, 0 } },
},
+ {
+ "load 64-bit immediate",
+ .u.insns_int = {
+ BPF_LD_IMM64(R1, 0x567800001234L),
+ BPF_MOV64_REG(R2, R1),
+ BPF_MOV64_REG(R3, R2),
+ BPF_ALU64_IMM(BPF_RSH, R2, 32),
+ BPF_ALU64_IMM(BPF_LSH, R3, 32),
+ BPF_ALU64_IMM(BPF_RSH, R3, 32),
+ BPF_ALU64_IMM(BPF_MOV, R0, 0),
+ BPF_JMP_IMM(BPF_JEQ, R2, 0x5678, 1),
+ BPF_EXIT_INSN(),
+ BPF_JMP_IMM(BPF_JEQ, R3, 0x1234, 1),
+ BPF_EXIT_INSN(),
+ BPF_ALU64_IMM(BPF_MOV, R0, 1),
+ BPF_EXIT_INSN(),
+ },
+ INTERNAL,
+ { },
+ { { 0, 1 } }
+ },
};
static struct net_device dev;
--
1.7.9.5
^ permalink raw reply related
* [PATCH v8 net-next 2/2] net: filter: split filter.h and expose eBPF to user space
From: Alexei Starovoitov @ 2014-08-27 20:37 UTC (permalink / raw)
To: David S. Miller
Cc: Ingo Molnar, Linus Torvalds, Andy Lutomirski, Steven Rostedt,
Daniel Borkmann, Chema Gonzalez, Eric Dumazet, Peter Zijlstra,
Brendan Gregg, Namhyung Kim, H. Peter Anvin, Andrew Morton,
Kees Cook, linux-api-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1409171833-6979-1-git-send-email-ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org>
allow user space to generate eBPF programs
uapi/linux/bpf.h: eBPF instruction set definition
linux/filter.h: the rest
This patch only moves macro definitions, but practically it freezes existing
eBPF instruction set, though new instructions can still be added in the future.
These eBPF definitions cannot go into uapi/linux/filter.h, since the names
may conflict with existing applications.
Full eBPF ISA description is in Documentation/networking/filter.txt
Signed-off-by: Alexei Starovoitov <ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org>
---
include/linux/filter.h | 56 +-------------------------------------
include/uapi/linux/Kbuild | 1 +
include/uapi/linux/bpf.h | 65 +++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 67 insertions(+), 55 deletions(-)
create mode 100644 include/uapi/linux/bpf.h
diff --git a/include/linux/filter.h b/include/linux/filter.h
index f3262b598262..3150666cd4b9 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -9,53 +9,7 @@
#include <linux/skbuff.h>
#include <linux/workqueue.h>
#include <uapi/linux/filter.h>
-
-/* Internally used and optimized filter representation with extended
- * instruction set based on top of classic BPF.
- */
-
-/* instruction classes */
-#define BPF_ALU64 0x07 /* alu mode in double word width */
-
-/* ld/ldx fields */
-#define BPF_DW 0x18 /* double word */
-#define BPF_XADD 0xc0 /* exclusive add */
-
-/* alu/jmp fields */
-#define BPF_MOV 0xb0 /* mov reg to reg */
-#define BPF_ARSH 0xc0 /* sign extending arithmetic shift right */
-
-/* change endianness of a register */
-#define BPF_END 0xd0 /* flags for endianness conversion: */
-#define BPF_TO_LE 0x00 /* convert to little-endian */
-#define BPF_TO_BE 0x08 /* convert to big-endian */
-#define BPF_FROM_LE BPF_TO_LE
-#define BPF_FROM_BE BPF_TO_BE
-
-#define BPF_JNE 0x50 /* jump != */
-#define BPF_JSGT 0x60 /* SGT is signed '>', GT in x86 */
-#define BPF_JSGE 0x70 /* SGE is signed '>=', GE in x86 */
-#define BPF_CALL 0x80 /* function call */
-#define BPF_EXIT 0x90 /* function return */
-
-/* Register numbers */
-enum {
- BPF_REG_0 = 0,
- BPF_REG_1,
- BPF_REG_2,
- BPF_REG_3,
- BPF_REG_4,
- BPF_REG_5,
- BPF_REG_6,
- BPF_REG_7,
- BPF_REG_8,
- BPF_REG_9,
- BPF_REG_10,
- __MAX_BPF_REG,
-};
-
-/* BPF has 10 general purpose 64-bit registers and stack frame. */
-#define MAX_BPF_REG __MAX_BPF_REG
+#include <uapi/linux/bpf.h>
/* ArgX, context and stack frame pointer register positions. Note,
* Arg1, Arg2, Arg3, etc are used as argument mappings of function
@@ -317,14 +271,6 @@ enum {
#define SK_RUN_FILTER(filter, ctx) \
(*filter->prog->bpf_func)(ctx, filter->prog->insnsi)
-struct bpf_insn {
- __u8 code; /* opcode */
- __u8 dst_reg:4; /* dest register */
- __u8 src_reg:4; /* source register */
- __s16 off; /* signed offset */
- __s32 imm; /* signed immediate constant */
-};
-
#ifdef CONFIG_COMPAT
/* A struct sock_filter is architecture independent. */
struct compat_sock_fprog {
diff --git a/include/uapi/linux/Kbuild b/include/uapi/linux/Kbuild
index 24e9033f8b3f..fb3f7b675229 100644
--- a/include/uapi/linux/Kbuild
+++ b/include/uapi/linux/Kbuild
@@ -67,6 +67,7 @@ header-y += bfs_fs.h
header-y += binfmts.h
header-y += blkpg.h
header-y += blktrace_api.h
+header-y += bpf.h
header-y += bpqether.h
header-y += bsg.h
header-y += btrfs.h
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
new file mode 100644
index 000000000000..479ed0b6be16
--- /dev/null
+++ b/include/uapi/linux/bpf.h
@@ -0,0 +1,65 @@
+/* Copyright (c) 2011-2014 PLUMgrid, http://plumgrid.com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of version 2 of the GNU General Public
+ * License as published by the Free Software Foundation.
+ */
+#ifndef _UAPI__LINUX_BPF_H__
+#define _UAPI__LINUX_BPF_H__
+
+#include <linux/types.h>
+
+/* Extended instruction set based on top of classic BPF */
+
+/* instruction classes */
+#define BPF_ALU64 0x07 /* alu mode in double word width */
+
+/* ld/ldx fields */
+#define BPF_DW 0x18 /* double word */
+#define BPF_XADD 0xc0 /* exclusive add */
+
+/* alu/jmp fields */
+#define BPF_MOV 0xb0 /* mov reg to reg */
+#define BPF_ARSH 0xc0 /* sign extending arithmetic shift right */
+
+/* change endianness of a register */
+#define BPF_END 0xd0 /* flags for endianness conversion: */
+#define BPF_TO_LE 0x00 /* convert to little-endian */
+#define BPF_TO_BE 0x08 /* convert to big-endian */
+#define BPF_FROM_LE BPF_TO_LE
+#define BPF_FROM_BE BPF_TO_BE
+
+#define BPF_JNE 0x50 /* jump != */
+#define BPF_JSGT 0x60 /* SGT is signed '>', GT in x86 */
+#define BPF_JSGE 0x70 /* SGE is signed '>=', GE in x86 */
+#define BPF_CALL 0x80 /* function call */
+#define BPF_EXIT 0x90 /* function return */
+
+/* Register numbers */
+enum {
+ BPF_REG_0 = 0,
+ BPF_REG_1,
+ BPF_REG_2,
+ BPF_REG_3,
+ BPF_REG_4,
+ BPF_REG_5,
+ BPF_REG_6,
+ BPF_REG_7,
+ BPF_REG_8,
+ BPF_REG_9,
+ BPF_REG_10,
+ __MAX_BPF_REG,
+};
+
+/* BPF has 10 general purpose 64-bit registers and stack frame. */
+#define MAX_BPF_REG __MAX_BPF_REG
+
+struct bpf_insn {
+ __u8 code; /* opcode */
+ __u8 dst_reg:4; /* dest register */
+ __u8 src_reg:4; /* source register */
+ __s16 off; /* signed offset */
+ __s32 imm; /* signed immediate constant */
+};
+
+#endif /* _UAPI__LINUX_BPF_H__ */
--
1.7.9.5
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox