Re: [PATCH net-next 1/1] netvsc: make sure and unregister datapath

[David Miller]

From: Stephen Hemminger 
Date: Mon,  7 Aug 2017 11:30:00 -0700

> Go back to switching datapath directly in the notifier callback.
> Otherwise datapath might not get switched on unregister.
> 
> No need for calling the NOTIFY_PEERS notifier since that is only for
> a gratitious ARP/ND packet; but that is not required with Hyper-V
> because both VF and synthetic NIC have the same MAC address.
> 
> Reported-by: Vitaly Kuznetsov 
> Fixes: 0c195567a8f6 ("netvsc: transparent VF management")
> Signed-off-by: Stephen Hemminger 

Applied, thanks Stephen.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH] binder: fix incorrect cmd to binder_stat_br

[Todd Kjos]

commit 26549d177410 ("binder: guarantee txn complete / errors delivered
in-order") passed the locally declared and undefined cmd
to binder_stat_br() which results in a bogus cmd field in a trace
event and BR stats are incremented incorrectly.

Change to use e->cmd which has been initialized.

Signed-off-by: Todd Kjos 
---
 drivers/android/binder.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/android/binder.c b/drivers/android/binder.c
index 9f95d7093f32..f34fcb513c64 100644
--- a/drivers/android/binder.c
+++ b/drivers/android/binder.c
@@ -3619,7 +3619,7 @@ static int binder_thread_read(struct binder_proc *proc,
e->cmd = BR_OK;
ptr += sizeof(uint32_t);
 
-   binder_stat_br(proc, thread, cmd);
+   binder_stat_br(proc, thread, e->cmd);
} break;
case BINDER_WORK_TRANSACTION_COMPLETE: {
binder_inner_proc_unlock(proc);
-- 
2.14.0.434.g98096fd7a8-goog

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [bug report] binder: guarantee txn complete / errors delivered in-order

[Greg KH]

On Tue, Aug 08, 2017 at 01:33:54PM -0700, Todd Kjos wrote:
> Thanks Dan. I just noticed your mail after getting back from vacation.
> 
> Greg- what is the best remedy for this? As Dan mentions, line 3622
> should be "e->cmd" instead of "cmd". This problem can't really cause
> any damage, but results in bogus "cmd" in trace output and can cause
> some stats to be incremented incorrectly. Should I resubmit that
> patch? Submit a new bugfix patch?

Submit a new bugfix on top of your existing patches as they are all now
committed to the tree.

thanks,

greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 10/18] staging: typec: fusb302: Add support for fcs,vbus-regulator-name device-property

[Hans de Goede]

Hi,

On 08/08/2017 04:42 PM, Mark Brown wrote:

On Tue, Aug 08, 2017 at 02:56:46PM +0100, Hans de Goede wrote:

Hi,


Please don't take things off-list unless there is a really strong reason
to do so.  Sending things to the list ensures that everyone gets a
chance to read and comment on things.


Sorry, that was unintentional I probably accidentally hit reply instead
of reply-to-all.

I've re-added the lists to the Cc.


On 08/08/2017 10:39 AM, Mark Brown wrote:

On Mon, Aug 07, 2017 at 09:20:05PM +0200, Hans de Goede wrote:



Why not?  This is just really standard usage of platform data.



Right, but in general in most cases we are trying to get rid of
platform data (where possible). So introducing new platform_data
is not going to be popular, but I agree that it likely is the
best solution here.


No, we aren't.  The majority of architectures are still platform data
only and x86 as you're finding uses it extensively along with ACPI.


Ok.


Alternatively the entry could additionally contain a provider_supply_name
so that we can make arbitrary consumer-dev-name + consumer-supply-name
provider-dev-name + provider-supply-name matches. That would probably
be more flexible then requiring the supply name to match.



I'm sorry but I can't follow what you mean here.  What do you mean by
"provider_supply_name"?



The current "const char *supply" in regulator_map is the supply name
passed to regulator_get, so the rdev_get_name requested by the consumer
(assuming no mapping is in place)


The name on the parent is *NOT* something anything else should
reference, it's just some internal documentation intended exclusively
for human consmption and can be overridden by the platforms.  It should
never be referenced by anything outside the device.


One regulator parent-device can register multiple regulator names, iow
multiple supplies, basically what I want to do is have the map
(when not using the regulator pointer) match the following 2 pairs:



dev_name + supply



regulator_parent_dev_name + rdev_get_name


Have your platform register identifiers that are useful within your
platform, don't rely on the drivers.


Ok, I need to think a bit about this. I think I've enough info to
come up with a new patch-set not introducing the fcs,vbus-regulator-name
device-property ugliness. But this is a side project and I'm rather busy with 
$dayjob
atm, so it may take a while for me to come up with a new patch.

Regards,

Hans
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [bug report] binder: guarantee txn complete / errors delivered in-order

[Todd Kjos]

Thanks Dan. I just noticed your mail after getting back from vacation.

Greg- what is the best remedy for this? As Dan mentions, line 3622
should be "e->cmd" instead of "cmd". This problem can't really cause
any damage, but results in bogus "cmd" in trace output and can cause
some stats to be incremented incorrectly. Should I resubmit that
patch? Submit a new bugfix patch?

-Todd


On Wed, Jul 19, 2017 at 12:00 PM, Dan Carpenter
 wrote:
> Hello Todd Kjos,
>
> The patch 26549d177410: "binder: guarantee txn complete / errors
> delivered in-order" from Jun 29, 2017, leads to the following static
> checker warning:
>
> drivers/android/binder.c:3622 binder_thread_read()
> error: uninitialized symbol 'cmd'.
>
> drivers/android/binder.c
>   3611  case BINDER_WORK_RETURN_ERROR: {
>   3612  struct binder_error *e = container_of(
>   3613  w, struct binder_error, work);
>   3614
>   3615  WARN_ON(e->cmd == BR_OK);
>   3616  binder_inner_proc_unlock(proc);
>   3617  if (put_user(e->cmd, (uint32_t __user *)ptr))
>   3618  return -EFAULT;
>   3619  e->cmd = BR_OK;
>   3620  ptr += sizeof(uint32_t);
>   3621
>   3622  binder_stat_br(proc, thread, cmd);
>  ^^^
>
> Uninitialized.  Probably e->cmd was intended?
>
>   3623  } break;
>   3624  case BINDER_WORK_TRANSACTION_COMPLETE: {
>   3625  binder_inner_proc_unlock(proc);
>   3626  cmd = BR_TRANSACTION_COMPLETE;
>   3627  if (put_user(cmd, (uint32_t __user *)ptr))
>   3628  return -EFAULT;
>
> regards,
> dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 3/3] ANDROID: binder: fix proc->tsk check.

[Greg KH]

On Tue, Aug 08, 2017 at 10:34:47AM -0700, John Stultz wrote:
> On Fri, Jul 28, 2017 at 4:56 AM, Martijn Coenen  wrote:
> > Commit c4ea41ba195d ("binder: use group leader instead of open thread")'
> > was incomplete and didn't update a check in binder_mmap(), causing all
> > mmap() calls into the binder driver to fail.
> >
> > Signed-off-by: Martijn Coenen 
> > ---
> >  drivers/android/binder.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/android/binder.c b/drivers/android/binder.c
> > index f7665c31feca..831cdd7d197d 100644
> > --- a/drivers/android/binder.c
> > +++ b/drivers/android/binder.c
> > @@ -3362,7 +3362,7 @@ static int binder_mmap(struct file *filp, struct 
> > vm_area_struct *vma)
> > const char *failure_string;
> > struct binder_buffer *buffer;
> >
> > -   if (proc->tsk != current)
> > +   if (proc->tsk != current->group_leader)
> > return -EINVAL;
> >
> > if ((vma->vm_end - vma->vm_start) > SZ_4M)
> 
> Tested-by: John Stultz 
> 
> As Amit already confirmed, this resolves the wifi and bluetooth
> regression I was seeing with Android using 4.13-rc2.
> 
> Though I've not seen it show up in Linus' tree yet, so I wanted to
> pester folks so it gets into 4.13-rc (its given me some slight grief
> trying to bisect down a separate issue).

I will queue this up in the next few days, I need to resolve the patches
that have been sent to me for this, sorry for the delay.

thanks,

greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 3/3] ANDROID: binder: fix proc->tsk check.

[John Stultz]

On Fri, Jul 28, 2017 at 4:56 AM, Martijn Coenen  wrote:
> Commit c4ea41ba195d ("binder: use group leader instead of open thread")'
> was incomplete and didn't update a check in binder_mmap(), causing all
> mmap() calls into the binder driver to fail.
>
> Signed-off-by: Martijn Coenen 
> ---
>  drivers/android/binder.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/android/binder.c b/drivers/android/binder.c
> index f7665c31feca..831cdd7d197d 100644
> --- a/drivers/android/binder.c
> +++ b/drivers/android/binder.c
> @@ -3362,7 +3362,7 @@ static int binder_mmap(struct file *filp, struct 
> vm_area_struct *vma)
> const char *failure_string;
> struct binder_buffer *buffer;
>
> -   if (proc->tsk != current)
> +   if (proc->tsk != current->group_leader)
> return -EINVAL;
>
> if ((vma->vm_end - vma->vm_start) > SZ_4M)

Tested-by: John Stultz 

As Amit already confirmed, this resolves the wifi and bluetooth
regression I was seeing with Android using 4.13-rc2.

Though I've not seen it show up in Linus' tree yet, so I wanted to
pester folks so it gets into 4.13-rc (its given me some slight grief
trying to bisect down a separate issue).

thanks
-john
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 1/3] staging: most: usb: constify usb_device_id

[Arvind Yadav]

usb_device_id are not supposed to change at runtime. All functions
working with usb_device_id provided by  work with
const usb_device_id. So mark the non-const structs as const.

Signed-off-by: Arvind Yadav 
---
 drivers/staging/most/hdm-usb/hdm_usb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/most/hdm-usb/hdm_usb.c 
b/drivers/staging/most/hdm-usb/hdm_usb.c
index d0f68cb..a9c3785 100644
--- a/drivers/staging/most/hdm-usb/hdm_usb.c
+++ b/drivers/staging/most/hdm-usb/hdm_usb.c
@@ -832,7 +832,7 @@ static const struct file_operations hdm_usb_fops = {
 /**
  * usb_device_id - ID table for HCD device probing
  */
-static struct usb_device_id usbid[] = {
+static const struct usb_device_id usbid[] = {
{ USB_DEVICE(USB_VENDOR_ID_SMSC, USB_DEV_ID_BRDG), },
{ USB_DEVICE(USB_VENDOR_ID_SMSC, USB_DEV_ID_OS81118), },
{ USB_DEVICE(USB_VENDOR_ID_SMSC, USB_DEV_ID_OS81119), },
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 2/3] staging: rtl8188eu: constify usb_device_id

[Arvind Yadav]

usb_device_id are not supposed to change at runtime. All functions
working with usb_device_id provided by  work with
const usb_device_id. So mark the non-const structs as const.

Signed-off-by: Arvind Yadav 
---
 drivers/staging/rtl8188eu/os_dep/usb_intf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/rtl8188eu/os_dep/usb_intf.c 
b/drivers/staging/rtl8188eu/os_dep/usb_intf.c
index 963235f..2ef71c3 100644
--- a/drivers/staging/rtl8188eu/os_dep/usb_intf.c
+++ b/drivers/staging/rtl8188eu/os_dep/usb_intf.c
@@ -32,7 +32,7 @@
 #define USB_VENDER_ID_REALTEK  0x0bda
 
 /* DID_USB_v916_20130116 */
-static struct usb_device_id rtw_usb_id_tbl[] = {
+static const struct usb_device_id rtw_usb_id_tbl[] = {
/*=== Realtek demoboard ===*/
{USB_DEVICE(USB_VENDER_ID_REALTEK, 0x8179)}, /* 8188EUS */
{USB_DEVICE(USB_VENDER_ID_REALTEK, 0x0179)}, /* 8188ETV */
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 3/3] staging: rtl8712: constify usb_device_id

[Arvind Yadav]

usb_device_id are not supposed to change at runtime. All functions
working with usb_device_id provided by  work with
const usb_device_id. So mark the non-const structs as const.

Signed-off-by: Arvind Yadav 
---
 drivers/staging/rtl8712/usb_intf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/rtl8712/usb_intf.c 
b/drivers/staging/rtl8712/usb_intf.c
index 897d462..b3e266b 100644
--- a/drivers/staging/rtl8712/usb_intf.c
+++ b/drivers/staging/rtl8712/usb_intf.c
@@ -47,7 +47,7 @@ static int r871xu_drv_init(struct usb_interface *pusb_intf,
 
 static void r871xu_dev_remove(struct usb_interface *pusb_intf);
 
-static struct usb_device_id rtl871x_usb_id_tbl[] = {
+static const struct usb_device_id rtl871x_usb_id_tbl[] = {
 
 /* RTL8188SU */
/* Realtek */
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 0/3] constify staging usb_device_id

[Arvind Yadav]

usb_device_id are not supposed to change at runtime. All functions
working with usb_device_id provided by  work with
const usb_device_id. So mark the non-const structs as const.

Arvind Yadav (3):
  [PATCH 1/3] staging: most: usb: constify usb_device_id
  [PATCH 2/3] staging: rtl8188eu: constify usb_device_id
  [PATCH 3/3] staging: rtl8712: constify usb_device_id

 drivers/staging/most/hdm-usb/hdm_usb.c  | 2 +-
 drivers/staging/rtl8188eu/os_dep/usb_intf.c | 2 +-
 drivers/staging/rtl8712/usb_intf.c  | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH net-next 0/1] netvsc: another VF datapath fix

[Stephen Hemminger]

The following would allow udev a chance at the device.



>From 37fb240a6107834c3dd3f57caede9d73b807f414 Mon Sep 17 00:00:00 2001
From: Stephen Hemminger 
Date: Tue, 8 Aug 2017 08:39:24 -0700
Subject: [PATCH] netvsc: delay setup of VF device

When VF device is discovered, delay bring it automatically up in
order to allow userspace to some simple changes (like renaming).

Signed-off-by: Stephen Hemminger 
---
 drivers/net/hyperv/hyperv_net.h |  2 +-
 drivers/net/hyperv/netvsc_drv.c | 11 ++-
 2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/net/hyperv/hyperv_net.h b/drivers/net/hyperv/hyperv_net.h
index d1ea99a12cf2..f620c90307ed 100644
--- a/drivers/net/hyperv/hyperv_net.h
+++ b/drivers/net/hyperv/hyperv_net.h
@@ -723,7 +723,7 @@ struct net_device_context {
/* State to manage the associated VF interface. */
struct net_device __rcu *vf_netdev;
struct netvsc_vf_pcpu_stats __percpu *vf_stats;
-   struct work_struct vf_takeover;
+   struct delayed_work vf_takeover;
 
/* 1: allocated, serial number is valid. 0: not allocated */
u32 vf_alloc;
diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index 7ebf0e10e62b..1dff160368a3 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -47,6 +47,7 @@
 
 #define RING_SIZE_MIN 64
 #define LINKCHANGE_INT (2 * HZ)
+#define VF_TAKEOVER_INT (HZ / 10)
 
 static int ring_size = 128;
 module_param(ring_size, int, S_IRUGO);
@@ -1570,7 +1571,7 @@ static int netvsc_vf_join(struct net_device *vf_netdev,
/* set slave flag before open to prevent IPv6 addrconf */
vf_netdev->flags |= IFF_SLAVE;
 
-   schedule_work(_ctx->vf_takeover);
+   schedule_delayed_work(_ctx->vf_takeover, VF_TAKEOVER_INT);
 
netdev_info(vf_netdev, "joined to %s\n", ndev->name);
return 0;
@@ -1608,12 +1609,12 @@ static void __netvsc_vf_setup(struct net_device *ndev,
 static void netvsc_vf_setup(struct work_struct *w)
 {
struct net_device_context *ndev_ctx
-   = container_of(w, struct net_device_context, vf_takeover);
+   = container_of(w, struct net_device_context, vf_takeover.work);
struct net_device *ndev = hv_get_drvdata(ndev_ctx->device_ctx);
struct net_device *vf_netdev;
 
if (!rtnl_trylock()) {
-   schedule_work(w);
+   schedule_delayed_work(_ctx->vf_takeover, 0);
return;
}
 
@@ -1717,7 +1718,7 @@ static int netvsc_unregister_vf(struct net_device 
*vf_netdev)
return NOTIFY_DONE;
 
net_device_ctx = netdev_priv(ndev);
-   cancel_work_sync(_device_ctx->vf_takeover);
+   cancel_delayed_work_sync(_device_ctx->vf_takeover);
 
netdev_info(ndev, "VF unregistering: %s\n", vf_netdev->name);
 
@@ -1759,7 +1760,7 @@ static int netvsc_probe(struct hv_device *dev,
 
spin_lock_init(_device_ctx->lock);
INIT_LIST_HEAD(_device_ctx->reconfig_events);
-   INIT_WORK(_device_ctx->vf_takeover, netvsc_vf_setup);
+   INIT_DELAYED_WORK(_device_ctx->vf_takeover, netvsc_vf_setup);
 
net_device_ctx->vf_stats
= netdev_alloc_pcpu_stats(struct netvsc_vf_pcpu_stats);
-- 
2.11.0

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH] staging: vboxvideo: make drm_fb_helper_funcs const

[Bhumika Goyal]

Make the structure const as it is only passed to the function
drm_fb_helper_prepare and the corresponding argument is of type
const.
Done using Coccinelle.

Signed-off-by: Bhumika Goyal 
---
 drivers/staging/vboxvideo/vbox_fb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/vboxvideo/vbox_fb.c 
b/drivers/staging/vboxvideo/vbox_fb.c
index bf66358..c386039 100644
--- a/drivers/staging/vboxvideo/vbox_fb.c
+++ b/drivers/staging/vboxvideo/vbox_fb.c
@@ -317,7 +317,7 @@ static int vboxfb_create(struct drm_fb_helper *helper,
return 0;
 }
 
-static struct drm_fb_helper_funcs vbox_fb_helper_funcs = {
+static const struct drm_fb_helper_funcs vbox_fb_helper_funcs = {
.fb_probe = vboxfb_create,
 };
 
-- 
1.9.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH net-next 0/1] netvsc: another VF datapath fix

[Vitaly Kuznetsov]

Stephen Hemminger  writes:

> On Tue, 08 Aug 2017 17:24:03 +0200
> Vitaly Kuznetsov  wrote:
>
>> Stephen Hemminger  writes:
>> 
>> > On Tue, 08 Aug 2017 16:03:56 +0200
>> > Vitaly Kuznetsov  wrote:
>> >  
>> >> Stephen Hemminger  writes:
>> >>   
>> >> > Previous fix was incomplete.
>> >> >
>> >> 
>> >> Not really related to this patch series (which btw fixes my issue,
>> >> thanks!), but I found one addition issue. Systemd fails to rename VF
>> >> interface:
>> >> 
>> >>  kernel: mlx4_core 0002:00:02.0 eth2: joined to eth1
>> >>  kernel: hv_netvsc 33b7a6f9-6736-451f-8fce-b382eaa50bee eth1: VF 
>> >> registering: eth2
>> >>  kernel: hv_netvsc 33b7a6f9-6736-451f-8fce-b382eaa50bee eth1: Data path 
>> >> switched to VF: eth2
>> >>  kernel: mlx4_en: eth2: Link Up
>> >>  NetworkManager[750]:   [1502200557.0821] device (eth2): link 
>> >> connected
>> >>  NetworkManager[750]:   [1502200557.1004] manager: (eth2): new 
>> >> Ethernet device (/org/freedesktop/NetworkManager/Devices/6)
>> >>  systemd-udevd[6942]: Error changing net interface name 'eth2' to 
>> >> 'enP2p0s2': Device or resource busy
>> >>  systemd-udevd[6942]: could not rename interface '6' from 'eth2' to 
>> >> 'enP2p0s2': Device or resource busy
>> >> 
>> >> With some debug added I figured out what's wrong: __netvsc_vf_setup()
>> >> does dev_open() which sets IFF_UP flag on the interface. When systemd
>> >> tries to rename the interface we get into dev_change_name() and this
>> >> fails with -EBUSY when (dev->flags & IFF_UP).
>> >> 
>> >> The issue is of less importance as we're not supposed to configure VF
>> >> interface now. However, we may still want to get a stable name for it.
>> >> 
>> >> Any idea how this can be fixed?  
>> >
>> > The problem is Network Manager should ignore the VF device. I don't run NM 
>> > on these
>> > interfaces because it causes more issues than it helps (dueling userspace).
>> >
>> > The driver needs to have slave track the master interface. Relying on 
>> > userspace
>> > to bring interface up leads to all the issues the bonding script had.
>> >
>> > One option would be to delay the work of bringing up the slave device to 
>> > allow
>> > small window for renaming to run.  
>> 
>> Actually, I tried removing 'if (dev->flags & IFF_UP)' check from
>> dev_change_name() and everything seems to work fine. The history of this
>> code predates git so I have no idea why it's forbiden to change names of
>> IFF_UP interfaces... I can send an RFC removing the check to figure out
>> the truth :-)
>
> That only works because NM by default brings everything up.
> Many users don't use NM on servers.

I'm not sure NetworkManager is related here: renaming is done by
systemd/udev according to the "predictable interface names" scheme:
https://www.freedesktop.org/wiki/Software/systemd/PredictableNetworkInterfaceNames/

Basically, systemd/udev tries to rename all new interfaces in the system
according to this scheme. And systemd/udev is ubiquitous nowdays.

I tried disabling NetworkManager in my VM and the behavior is not any
different:

 kernel: mlx4_core 0002:00:02.0 eth2: joined to eth1
 kernel: hv_netvsc 33b7a6f9-6736-451f-8fce-b382eaa50bee eth1: VF registering: 
eth2
 kernel: mlx4_en: eth2: Link Up
 kernel: hv_netvsc 33b7a6f9-6736-451f-8fce-b382eaa50bee eth1: Data path 
switched to VF: eth2
 systemd-udevd[1785]: Error changing net interface name 'eth2' to 'enP2p0s2': 
Device or resource busy
 systemd-udevd[1785]: could not rename interface '5' from 'eth2' to 'enP2p0s2': 
Device or resource busy

The 'if (dev->flags & IFF_UP)' check in dev_change_name() function
I mentioned prevents renaming interfaces which are already up but I
don't know an obvious reason for it to exist.

-- 
  Vitaly
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH net-next 0/1] netvsc: another VF datapath fix

[Stephen Hemminger]

On Tue, 08 Aug 2017 17:24:03 +0200
Vitaly Kuznetsov  wrote:

> Stephen Hemminger  writes:
> 
> > On Tue, 08 Aug 2017 16:03:56 +0200
> > Vitaly Kuznetsov  wrote:
> >  
> >> Stephen Hemminger  writes:
> >>   
> >> > Previous fix was incomplete.
> >> >
> >> 
> >> Not really related to this patch series (which btw fixes my issue,
> >> thanks!), but I found one addition issue. Systemd fails to rename VF
> >> interface:
> >> 
> >>  kernel: mlx4_core 0002:00:02.0 eth2: joined to eth1
> >>  kernel: hv_netvsc 33b7a6f9-6736-451f-8fce-b382eaa50bee eth1: VF 
> >> registering: eth2
> >>  kernel: hv_netvsc 33b7a6f9-6736-451f-8fce-b382eaa50bee eth1: Data path 
> >> switched to VF: eth2
> >>  kernel: mlx4_en: eth2: Link Up
> >>  NetworkManager[750]:   [1502200557.0821] device (eth2): link 
> >> connected
> >>  NetworkManager[750]:   [1502200557.1004] manager: (eth2): new 
> >> Ethernet device (/org/freedesktop/NetworkManager/Devices/6)
> >>  systemd-udevd[6942]: Error changing net interface name 'eth2' to 
> >> 'enP2p0s2': Device or resource busy
> >>  systemd-udevd[6942]: could not rename interface '6' from 'eth2' to 
> >> 'enP2p0s2': Device or resource busy
> >> 
> >> With some debug added I figured out what's wrong: __netvsc_vf_setup()
> >> does dev_open() which sets IFF_UP flag on the interface. When systemd
> >> tries to rename the interface we get into dev_change_name() and this
> >> fails with -EBUSY when (dev->flags & IFF_UP).
> >> 
> >> The issue is of less importance as we're not supposed to configure VF
> >> interface now. However, we may still want to get a stable name for it.
> >> 
> >> Any idea how this can be fixed?  
> >
> > The problem is Network Manager should ignore the VF device. I don't run NM 
> > on these
> > interfaces because it causes more issues than it helps (dueling userspace).
> >
> > The driver needs to have slave track the master interface. Relying on 
> > userspace
> > to bring interface up leads to all the issues the bonding script had.
> >
> > One option would be to delay the work of bringing up the slave device to 
> > allow
> > small window for renaming to run.  
> 
> Actually, I tried removing 'if (dev->flags & IFF_UP)' check from
> dev_change_name() and everything seems to work fine. The history of this
> code predates git so I have no idea why it's forbiden to change names of
> IFF_UP interfaces... I can send an RFC removing the check to figure out
> the truth :-)

That only works because NM by default brings everything up.
Many users don't use NM on servers.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH net-next 0/1] netvsc: another VF datapath fix

[Vitaly Kuznetsov]

Stephen Hemminger  writes:

> On Tue, 08 Aug 2017 16:03:56 +0200
> Vitaly Kuznetsov  wrote:
>
>> Stephen Hemminger  writes:
>> 
>> > Previous fix was incomplete.
>> >  
>> 
>> Not really related to this patch series (which btw fixes my issue,
>> thanks!), but I found one addition issue. Systemd fails to rename VF
>> interface:
>> 
>>  kernel: mlx4_core 0002:00:02.0 eth2: joined to eth1
>>  kernel: hv_netvsc 33b7a6f9-6736-451f-8fce-b382eaa50bee eth1: VF 
>> registering: eth2
>>  kernel: hv_netvsc 33b7a6f9-6736-451f-8fce-b382eaa50bee eth1: Data path 
>> switched to VF: eth2
>>  kernel: mlx4_en: eth2: Link Up
>>  NetworkManager[750]:   [1502200557.0821] device (eth2): link connected
>>  NetworkManager[750]:   [1502200557.1004] manager: (eth2): new 
>> Ethernet device (/org/freedesktop/NetworkManager/Devices/6)
>>  systemd-udevd[6942]: Error changing net interface name 'eth2' to 
>> 'enP2p0s2': Device or resource busy
>>  systemd-udevd[6942]: could not rename interface '6' from 'eth2' to 
>> 'enP2p0s2': Device or resource busy
>> 
>> With some debug added I figured out what's wrong: __netvsc_vf_setup()
>> does dev_open() which sets IFF_UP flag on the interface. When systemd
>> tries to rename the interface we get into dev_change_name() and this
>> fails with -EBUSY when (dev->flags & IFF_UP).
>> 
>> The issue is of less importance as we're not supposed to configure VF
>> interface now. However, we may still want to get a stable name for it.
>> 
>> Any idea how this can be fixed?
>
> The problem is Network Manager should ignore the VF device. I don't run NM on 
> these
> interfaces because it causes more issues than it helps (dueling userspace).
>
> The driver needs to have slave track the master interface. Relying on 
> userspace
> to bring interface up leads to all the issues the bonding script had.
>
> One option would be to delay the work of bringing up the slave device to allow
> small window for renaming to run.

Actually, I tried removing 'if (dev->flags & IFF_UP)' check from
dev_change_name() and everything seems to work fine. The history of this
code predates git so I have no idea why it's forbiden to change names of
IFF_UP interfaces... I can send an RFC removing the check to figure out
the truth :-)

-- 
  Vitaly
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH net-next 0/1] netvsc: another VF datapath fix

[Stephen Hemminger]

On Tue, 08 Aug 2017 16:03:56 +0200
Vitaly Kuznetsov  wrote:

> Stephen Hemminger  writes:
> 
> > Previous fix was incomplete.
> >  
> 
> Not really related to this patch series (which btw fixes my issue,
> thanks!), but I found one addition issue. Systemd fails to rename VF
> interface:
> 
>  kernel: mlx4_core 0002:00:02.0 eth2: joined to eth1
>  kernel: hv_netvsc 33b7a6f9-6736-451f-8fce-b382eaa50bee eth1: VF registering: 
> eth2
>  kernel: hv_netvsc 33b7a6f9-6736-451f-8fce-b382eaa50bee eth1: Data path 
> switched to VF: eth2
>  kernel: mlx4_en: eth2: Link Up
>  NetworkManager[750]:   [1502200557.0821] device (eth2): link connected
>  NetworkManager[750]:   [1502200557.1004] manager: (eth2): new Ethernet 
> device (/org/freedesktop/NetworkManager/Devices/6)
>  systemd-udevd[6942]: Error changing net interface name 'eth2' to 'enP2p0s2': 
> Device or resource busy
>  systemd-udevd[6942]: could not rename interface '6' from 'eth2' to 
> 'enP2p0s2': Device or resource busy
> 
> With some debug added I figured out what's wrong: __netvsc_vf_setup()
> does dev_open() which sets IFF_UP flag on the interface. When systemd
> tries to rename the interface we get into dev_change_name() and this
> fails with -EBUSY when (dev->flags & IFF_UP).
> 
> The issue is of less importance as we're not supposed to configure VF
> interface now. However, we may still want to get a stable name for it.
> 
> Any idea how this can be fixed?

The problem is Network Manager should ignore the VF device. I don't run NM on 
these
interfaces because it causes more issues than it helps (dueling userspace).

The driver needs to have slave track the master interface. Relying on userspace
to bring interface up leads to all the issues the bonding script had.

One option would be to delay the work of bringing up the slave device to allow
small window for renaming to run.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH net-next 0/1] netvsc: another VF datapath fix

[Vitaly Kuznetsov]

Stephen Hemminger  writes:

> Previous fix was incomplete.
>

Not really related to this patch series (which btw fixes my issue,
thanks!), but I found one addition issue. Systemd fails to rename VF
interface:

 kernel: mlx4_core 0002:00:02.0 eth2: joined to eth1
 kernel: hv_netvsc 33b7a6f9-6736-451f-8fce-b382eaa50bee eth1: VF registering: 
eth2
 kernel: hv_netvsc 33b7a6f9-6736-451f-8fce-b382eaa50bee eth1: Data path 
switched to VF: eth2
 kernel: mlx4_en: eth2: Link Up
 NetworkManager[750]:   [1502200557.0821] device (eth2): link connected
 NetworkManager[750]:   [1502200557.1004] manager: (eth2): new Ethernet 
device (/org/freedesktop/NetworkManager/Devices/6)
 systemd-udevd[6942]: Error changing net interface name 'eth2' to 'enP2p0s2': 
Device or resource busy
 systemd-udevd[6942]: could not rename interface '6' from 'eth2' to 'enP2p0s2': 
Device or resource busy

With some debug added I figured out what's wrong: __netvsc_vf_setup()
does dev_open() which sets IFF_UP flag on the interface. When systemd
tries to rename the interface we get into dev_change_name() and this
fails with -EBUSY when (dev->flags & IFF_UP).

The issue is of less importance as we're not supposed to configure VF
interface now. However, we may still want to get a stable name for it.

Any idea how this can be fixed?

-- 
  Vitaly
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH] hv_set_ifconfig.sh double check before setting ip

[Eduardo Otubo]

This patch fixes the behavior of the hv_set_ifconfig script when setting
the interface ip. Sometimes the interface has already been configured by
network daemon, in this case hv_set_ifconfig causes "RTNETLINK: file
exists error"; in order to avoid this error this patch makes sure double
checks the interface before trying anything.

Signed-off-by: Eduardo Otubo 
---
 tools/hv/hv_set_ifconfig.sh | 29 ++---
 1 file changed, 18 insertions(+), 11 deletions(-)

diff --git a/tools/hv/hv_set_ifconfig.sh b/tools/hv/hv_set_ifconfig.sh
index 735aafd64a3f..ba5e4aa4efe2 100755
--- a/tools/hv/hv_set_ifconfig.sh
+++ b/tools/hv/hv_set_ifconfig.sh
@@ -46,19 +46,26 @@
 # is expected to return the configuration that is set via the SET
 # call.
 #
+interface=$(echo $1 | awk -F - '{ print $2 }')
 
+current_ip=$(ip addr show $interface|grep "inet ");
+config_file_ip=$(grep IPADDR $1|cut -d"=" -f2);
 
+current_ipv6=$(ip addr show $interface|grep "inet6 ");
+config_file_ipv6=$(grep IPV6ADDR $1|cut -d"=" -f2);
+config_file_ipv6_netmask=$(grep IPV6NETMASK $1|cut -d"=" -f2);
+config_file_ipv6=${config_file_ipv6}/${config_file_ipv6_netmask};
 
-echo "IPV6INIT=yes" >> $1
-echo "NM_CONTROLLED=no" >> $1
-echo "PEERDNS=yes" >> $1
-echo "ONBOOT=yes" >> $1
-
-
-cp $1 /etc/sysconfig/network-scripts/
-
+# only set the IP if it's not configured yet
+if [[ $(test "${current_ip#*$config_file_ip}") == "$config_file_ip" \
+|| $(test "${current_ipv6#*$config_file_ipv6}") == "$current_ipv6" ]]; then
+echo "IPV6INIT=yes" >> $1
+echo "NM_CONTROLLED=no" >> $1
+echo "PEERDNS=yes" >> $1
+echo "ONBOOT=yes" >> $1
 
-interface=$(echo $1 | awk -F - '{ print $2 }')
+cp $1 /etc/sysconfig/network-scripts/
 
-/sbin/ifdown $interface 2>/dev/null
-/sbin/ifup $interface 2>/dev/null
+/sbin/ifdown $interface 2>/dev/null
+/sbin/ifup $interface 2>/dev/null
+fi
-- 
2.13.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] media: staging: atomisp: sh_css_calloc shall return a pointer to the allocated space

[Sakari Ailus]

On Wed, Aug 02, 2017 at 01:11:06AM -0700, Joe Perches wrote:
> On Wed, 2017-08-02 at 18:00 +1000, Sergei A. Trusov wrote:
> > The calloc function returns either a null pointer or a pointer to the
> > allocated space. Add the second case that is missed.
> 
> gads.
> 
> Bug added by commit da22013f7df4 ("atomisp: remove indirection from
> sh_css_malloc")
> 
> These wrappers should really be deleted.

Applied, with the appropriate Fixes: header.

Thanks!

-- 
Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] Remove explicit return type cast

[hari prasath]

On 3 August 2017 at 18:53, Dan Carpenter  wrote:
> On Thu, Aug 03, 2017 at 06:23:54PM +0530, hari prasath wrote:
>> On 3 August 2017 at 11:52, kbuild test robot  wrote:
>> > Hi Hari,
>> >
>> > [auto build test WARNING on staging/staging-testing]
>> > [also build test WARNING on next-20170802]
>> > [cannot apply to v4.13-rc3]
>> > [if your patch is applied to the wrong git tree, please drop us a note to 
>> > help improve the system]
>> >
>> > url:
>> > https://github.com/0day-ci/linux/commits/Hari-Prasath/Remove-explicit-return-type-cast/20170803-080312
>> > config: blackfin-allyesconfig (attached as .config)
>> > compiler: bfin-uclinux-gcc (GCC) 6.2.0
>> > reproduce:
>> > wget 
>> > https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross 
>> > -O ~/bin/make.cross
>> > chmod +x ~/bin/make.cross
>> > # save the attached .config to linux build tree
>> > make.cross ARCH=blackfin
>> >
>>
>> >> I tried these steps, it's giving me build error as below.
>>
>
> You don't need to cross compile on blackfin to get the warning.  Just
> use the normal compiler.
>
> regards,
> dan carpenter
>

> Sorry, I had sent the patch in a hurry. Yes the warning is true. I will try 
> to come up with a v2 of the patch without any warnings. As of now this can be 
> discarded.
>  thanks,
>  hari prasath



-- 
Regards,
G.E.Hari Prasath
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: media: atomisp: use kvmalloc/kvzalloc

[Alan Cox]

On Mon, 2017-08-07 at 21:44 +0800, Geliang Tang wrote:
> Use kvmalloc()/kvzalloc() instead of atomisp_kernel_malloc()
> /atomisp_kernel_zalloc().
> 
> Signed-off-by: Geliang Tang 

Definitely better now we have kvmalloc/kvzalloc.

Thanks

Alan

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH] staging: bcm2835-audio: Fix memory corruption

[Phil Elwell]

I'm all for fixing memory leaks, but freeing a block while it is still
being used is a recipe for hard-to-debug kernel exceptions.

1) There is already a vchi method for freeing the instance, so use it.
2) Only call it on error, and then only before initted is false.

Signed-off-by: Phil Elwell 
Fixes: 0adbfd4694c2 ("staging: bcm2835-audio: fix memory leak in 
bcm2835_audio_open_connection()")
---
 drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c 
b/drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c
index 5f3d8f2..89f96f3 100644
--- a/drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c
+++ b/drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c
@@ -409,6 +409,7 @@ static int bcm2835_audio_open_connection(struct 
bcm2835_alsa_stream *alsa_stream
LOG_ERR("%s: failed to connect VCHI instance 
(ret=%d)\n",
__func__, ret);
 
+   vchi_disconnect(vchi_instance);
ret = -EIO;
goto err_free_mem;
}
@@ -431,7 +432,6 @@ static int bcm2835_audio_open_connection(struct 
bcm2835_alsa_stream *alsa_stream
LOG_DBG(" success !\n");
ret = 0;
 err_free_mem:
-   kfree(vchi_instance);
 
return ret;
 }
-- 
1.9.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: pi433: style fix - space after asterisk

[Marcus Wolf]

Reviewed-by: Marcus Wolf 

Thank you Marcin!

> Marcin Ciupak  hat am 8. August 2017 um 15:54
> geschrieben:
>
>
> This patch is intended to fix coding style issues in order to comply
> with kernel coding style guide as requested by TODO file.
>
> It fixes the following checkpatch.pl error:
>
> ERROR: "foo * bar" should be "foo *bar"
>
> Note:
> "WARNING: line over 80 characters" remains valid here and could be fixed
> by another patch.
>
> Signed-off-by: Marcin Ciupak 
> ---
> drivers/staging/pi433/rf69.c | 2 +-
> drivers/staging/pi433/rf69.h | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/pi433/rf69.c b/drivers/staging/pi433/rf69.c
> index f83523e3395d..9f4e7173c688 100644
> --- a/drivers/staging/pi433/rf69.c
> +++ b/drivers/staging/pi433/rf69.c
> @@ -740,7 +740,7 @@ int rf69_set_sync_values(struct spi_device *spi, u8
> syncValues[8])
> return retval;
> }
>
> -int rf69_set_packet_format(struct spi_device * spi, enum packetFormat
> packetFormat)
> +int rf69_set_packet_format(struct spi_device *spi, enum packetFormat
> packetFormat)
> {
> #ifdef DEBUG
> dev_dbg(>dev, "set: packet format");
> diff --git a/drivers/staging/pi433/rf69.h b/drivers/staging/pi433/rf69.h
> index b81e0762032e..fbccaae51c61 100644
> --- a/drivers/staging/pi433/rf69.h
> +++ b/drivers/staging/pi433/rf69.h
> @@ -61,7 +61,7 @@ int rf69_set_fifo_fill_condition(struct spi_device *spi,
> enum fifoFillCondition
> int rf69_set_sync_size(struct spi_device *spi, u8 sync_size);
> int rf69_set_sync_tolerance(struct spi_device *spi, u8 syncTolerance);
> int rf69_set_sync_values(struct spi_device *spi, u8 syncValues[8]);
> -int rf69_set_packet_format(struct spi_device * spi, enum packetFormat
> packetFormat);
> +int rf69_set_packet_format(struct spi_device *spi, enum packetFormat
> packetFormat);
> int rf69_set_crc_enable(struct spi_device *spi, enum optionOnOff optionOnOff);
> int rf69_set_adressFiltering(struct spi_device *spi, enum addressFiltering
> addressFiltering);
> int rf69_set_payload_length(struct spi_device *spi, u8 payloadLength);
> --
> 2.13.0
>
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [RESEND PATCH] staging: vboxvideo: remove dead gamma lut code

[Peter Rosin]

On 2017-08-07 11:21, Daniel Vetter wrote:
> On Fri, Aug 04, 2017 at 12:45:06PM +0200, Peter Rosin wrote:
>> The redundant fb helpers .load_lut, .gamma_set and .gamma_get are
>> no longer used. Remove the dead code that was not doing anything
>> sensible anyway.
>>
>> Signed-off-by: Peter Rosin 
>> ---
>>  drivers/staging/vboxvideo/vbox_fb.c   | 15 ---
>>  drivers/staging/vboxvideo/vbox_mode.c |  5 -
>>  2 files changed, 20 deletions(-)
>>
>> [This time with an improved Cc list, sorry for the noise. For new
>>  people, please refer to https://lkml.org/lkml/2017/7/13/593 for
>>  context]
>>
>> Hi Daniel,
>>
>> Here it goes, but do you really need me to resend v5 14/14?
> 
> Well it's not yet on dri-devel, but our pre-merge CI bots can't read such
> instructions. They expect a clean new series that applies cleanly, as a
> top-post, when resending stuff.

Ok, noted for the next time. These things seem to vary from subsystem to
subsystem, so it's not trivial to get it right w/o investing serious time
looking things up. Anyway, sorry about the trouble.

> Anyway, applied.

Thanks!

Cheers,
Peter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH] staging: pi433: style fix - space after asterisk

[Marcin Ciupak]

This patch is intended to fix coding style issues in order to comply
with kernel coding style guide as requested by TODO file.

It fixes the following checkpatch.pl error:

ERROR: "foo * bar" should be "foo *bar"

Note:
"WARNING: line over 80 characters" remains valid here and could be fixed
by another patch.

Signed-off-by: Marcin Ciupak 
---
 drivers/staging/pi433/rf69.c | 2 +-
 drivers/staging/pi433/rf69.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/pi433/rf69.c b/drivers/staging/pi433/rf69.c
index f83523e3395d..9f4e7173c688 100644
--- a/drivers/staging/pi433/rf69.c
+++ b/drivers/staging/pi433/rf69.c
@@ -740,7 +740,7 @@ int rf69_set_sync_values(struct spi_device *spi, u8 
syncValues[8])
return retval;
 }
 
-int rf69_set_packet_format(struct spi_device * spi, enum packetFormat 
packetFormat)
+int rf69_set_packet_format(struct spi_device *spi, enum packetFormat 
packetFormat)
 {
#ifdef DEBUG
dev_dbg(>dev, "set: packet format");
diff --git a/drivers/staging/pi433/rf69.h b/drivers/staging/pi433/rf69.h
index b81e0762032e..fbccaae51c61 100644
--- a/drivers/staging/pi433/rf69.h
+++ b/drivers/staging/pi433/rf69.h
@@ -61,7 +61,7 @@ int rf69_set_fifo_fill_condition(struct spi_device *spi, enum 
fifoFillCondition
 int rf69_set_sync_size(struct spi_device *spi, u8 sync_size);
 int rf69_set_sync_tolerance(struct spi_device *spi, u8 syncTolerance);
 int rf69_set_sync_values(struct spi_device *spi, u8 syncValues[8]);
-int rf69_set_packet_format(struct spi_device * spi, enum packetFormat 
packetFormat);
+int rf69_set_packet_format(struct spi_device *spi, enum packetFormat 
packetFormat);
 int rf69_set_crc_enable(struct spi_device *spi, enum optionOnOff optionOnOff);
 int rf69_set_adressFiltering(struct spi_device *spi, enum addressFiltering 
addressFiltering);
 int rf69_set_payload_length(struct spi_device *spi, u8 payloadLength);
-- 
2.13.0

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] Staging: comedi: addi_apci_3501: fixed a CamelCase coding style issue

[徐細菌]

> This patch is incomplete (the 'tsk_Current' member is also used in
> "drivers/staging/comedi/addi-data/hwdrv_apci3501.c" which is #include'd by
> "addi_apci_3501.c") and out-of-date (the 'tsk_Current' member was removed in
> kernel version 4.9 by commit a6672530f6fc ("staging: comedi: addi_apci_3501:
> remove timer/counter subdevice support").
>
Got it, great thanks for your detailed reply.


2017-08-08 17:40 GMT+08:00 Ian Abbott :
> On 08/08/17 08:42, Chi-Chun Hsu wrote:
>>
>> Fixed a coding style issue.
>>
>> Signed-off-by: Chi-Chun Hsu 
>> ---
>>   drivers/staging/comedi/drivers/addi_apci_3501.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/staging/comedi/drivers/addi_apci_3501.c
>> b/drivers/staging/comedi/drivers/addi_apci_3501.c
>> index 40ff914..b2f6712 100644
>> --- a/drivers/staging/comedi/drivers/addi_apci_3501.c
>> +++ b/drivers/staging/comedi/drivers/addi_apci_3501.c
>> @@ -68,7 +68,7 @@
>>   struct apci3501_private {
>> unsigned long amcc;
>> unsigned long tcw;
>> -   struct task_struct *tsk_Current;
>> +   struct task_struct *tsk_current;
>> unsigned char timer_mode;
>>   };
>>   @@ -273,7 +273,7 @@ static irqreturn_t apci3501_interrupt(int irq, void
>> *d)
>> }
>> /* Enable Interrupt Send a signal to from kernel to user space */
>> -   send_sig(SIGIO, devpriv->tsk_Current, 0);
>> +   send_sig(SIGIO, devpriv->tsk_current, 0);
>> ctrl = inl(devpriv->tcw + ADDI_TCW_CTRL_REG);
>> ctrl &= ~(ADDI_TCW_CTRL_GATE | ADDI_TCW_CTRL_TRIG |
>>   ADDI_TCW_CTRL_IRQ_ENA);
>>
>
> This patch is incomplete (the 'tsk_Current' member is also used in
> "drivers/staging/comedi/addi-data/hwdrv_apci3501.c" which is #include'd by
> "addi_apci_3501.c") and out-of-date (the 'tsk_Current' member was removed in
> kernel version 4.9 by commit a6672530f6fc ("staging: comedi: addi_apci_3501:
> remove timer/counter subdevice support").
>
> --
> -=( Ian Abbott @ MEV Ltd.E-mail:  )=-
> -=(  Web: http://www.mev.co.uk/  )=-
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 3/6] staging: atomisp: constify video_subdev structures

[Julia Lawall]

These structures are both stored in fields of v4l2_subdev_ops
structures, all of which are const, so these structures can be
const as well.

Done with the help of Coccinelle.

Signed-off-by: Julia Lawall 

---
 drivers/staging/media/atomisp/i2c/ap1302.c  |2 +-
 drivers/staging/media/atomisp/i2c/mt9m114.c |2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/media/atomisp/i2c/ap1302.c 
b/drivers/staging/media/atomisp/i2c/ap1302.c
index bacffbe..de687c6 100644
--- a/drivers/staging/media/atomisp/i2c/ap1302.c
+++ b/drivers/staging/media/atomisp/i2c/ap1302.c
@@ -1098,7 +1098,7 @@ static long ap1302_ioctl(struct v4l2_subdev *sd, unsigned 
int cmd, void *arg)
},
 };
 
-static struct v4l2_subdev_sensor_ops ap1302_sensor_ops = {
+static const struct v4l2_subdev_sensor_ops ap1302_sensor_ops = {
.g_skip_frames  = ap1302_g_skip_frames,
 };
 
diff --git a/drivers/staging/media/atomisp/i2c/mt9m114.c 
b/drivers/staging/media/atomisp/i2c/mt9m114.c
index 448e072..36a0636 100644
--- a/drivers/staging/media/atomisp/i2c/mt9m114.c
+++ b/drivers/staging/media/atomisp/i2c/mt9m114.c
@@ -1806,7 +1806,7 @@ static int mt9m114_g_skip_frames(struct v4l2_subdev *sd, 
u32 *frames)
.g_frame_interval = mt9m114_g_frame_interval,
 };
 
-static struct v4l2_subdev_sensor_ops mt9m114_sensor_ops = {
+static const struct v4l2_subdev_sensor_ops mt9m114_sensor_ops = {
.g_skip_frames  = mt9m114_g_skip_frames,
 };
 

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 3/3] drm/etnaviv: add thermal dependency

[Lucas Stach]

Hi Arnd,

Am Mittwoch, den 26.07.2017, 15:53 +0200 schrieb Arnd Bergmann:
> When CONFIG_THERMAL is enabled as a loadable module, and etnaviv is
> built-in, we get a link error:
> 
> drivers/gpu/drm/etnaviv/etnaviv_gpu.o: In function `etnaviv_gpu_bind':
> etnaviv_gpu.c:(.text.etnaviv_gpu_bind+0x34): undefined reference to 
> `thermal_of_cooling_device_register'
> etnaviv_gpu.c:(.text.etnaviv_gpu_bind+0xe0): undefined reference to 
> `thermal_cooling_device_unregister'
> drivers/gpu/drm/etnaviv/etnaviv_gpu.o: In function `etnaviv_gpu_unbind':
> etnaviv_gpu.c:(.text.etnaviv_gpu_unbind+0xf0): undefined reference to 
> `thermal_cooling_device_unregister'
> 
> This adds a Kconfig dependency to prevent etnaviv from being built-in
> with CONFIG_THERMAL=m, while still allowing the valid configurations.
> Unfortunately, simply adding the dependency here breaks Kconfig through
> a dependency loop involving lots of symbols all the way until ACPI_VIDEO,
> which is the only video driver that explicitly selects 'THERMAL'. Turning
> this into a 'depends on' addresses the problem.
> For completeness, I'm also removing the redundant 'select THERMAL'
> from INTEL_MENLOW, so no other driver uses that statement.
> 
> Fixes: bcdfb5e56dc5 ("drm/etnaviv: add etnaviv cooling device")
> Signed-off-by: Arnd Bergmann 
> ---
> v2: spend more thought on ACPI_VIDEO dependencies, as we need another
> patch before this.
> ---
>  drivers/acpi/Kconfig| 2 +-
>  drivers/gpu/drm/etnaviv/Kconfig | 1 +
>  drivers/platform/x86/Kconfig| 1 -

I would like to take this patch, but I'm not sure why it includes x86
and ACPI hunks.

Regards,
Lucas

>  3 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
> index a8f5a40e2914..1282b2936164 100644
> --- a/drivers/acpi/Kconfig
> +++ b/drivers/acpi/Kconfig
> @@ -184,7 +184,7 @@ config ACPI_VIDEO
>   tristate "Video"
>   depends on X86
>   depends on INPUT
> - select THERMAL
> + depends on THERMAL
>   select BACKLIGHT_CLASS_DEVICE
>   select BACKLIGHT_LCD_SUPPORT
>   default y
> diff --git a/drivers/gpu/drm/etnaviv/Kconfig b/drivers/gpu/drm/etnaviv/Kconfig
> index 71cee4e9fefb..1d6c744e7924 100644
> --- a/drivers/gpu/drm/etnaviv/Kconfig
> +++ b/drivers/gpu/drm/etnaviv/Kconfig
> @@ -3,6 +3,7 @@ config DRM_ETNAVIV
>   tristate "ETNAVIV (DRM support for Vivante GPU IP cores)"
>   depends on DRM
>   depends on ARCH_MXC || ARCH_DOVE || (ARM && COMPILE_TEST)
> + depends on THERMAL || THERMAL=n
>   depends on MMU
>   select SHMEM
>   select SYNC_FILE
> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
> index 2d9fb46a8d11..d9238e9ff54a 100644
> --- a/drivers/platform/x86/Kconfig
> +++ b/drivers/platform/x86/Kconfig
> @@ -532,7 +532,6 @@ config SENSORS_HDAPS
>  config INTEL_MENLOW
>   tristate "Thermal Management driver for Intel menlow platform"
>   depends on ACPI_THERMAL
> - select THERMAL
>   ---help---
> ACPI thermal management enhancement driver on
> Intel Menlow platform.


___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH] binder: let ANDROID_BINDER_IPC_32BIT be selectable on 32bit ARM

[Jisheng Zhang]

As noted in commit d0bdff0db809 ("staging: Fix build issues with new
binder API"), we can add back the choice for 32bit ARM "once a 64bit
__get_user_asm_* implementation is merged." Commit e38361d032f1 ("ARM:
8091/2: add get_user() support for 8 byte types") has added the
support, so it's time to let ANDROID_BINDER_IPC_32BIT be selectable on
32bit ARM

Signed-off-by: Jisheng Zhang 
---
 drivers/android/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/android/Kconfig b/drivers/android/Kconfig
index 832e885349b1..aca5dc30b97b 100644
--- a/drivers/android/Kconfig
+++ b/drivers/android/Kconfig
@@ -32,7 +32,7 @@ config ANDROID_BINDER_DEVICES
  therefore logically separated from the other devices.
 
 config ANDROID_BINDER_IPC_32BIT
-   bool
+   bool "Use old (Android 4.4 and earlier) 32-bit binder API"
depends on !64BIT && ANDROID_BINDER_IPC
default y
---help---
-- 
2.14.0

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 2/5] [media] atomisp: constify videobuf_queue_ops structures

[Sakari Ailus]

On Fri, Aug 04, 2017 at 02:09:45PM +0200, Julia Lawall wrote:
> These videobuf_queue_ops structures are only passed as the second
> argument to videobuf_queue_vmalloc_init, which is declared as const.
> Thus the videobuf_queue_ops structures themselves can be const.
> 
> Done with the help of Coccinelle.

Thanks. I'm picking this one up (for atomisp).

-- 
Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 10/18] staging: typec: fusb302: Add support for fcs,vbus-regulator-name device-property

[Mark Brown]

On Mon, Aug 07, 2017 at 09:20:05PM +0200, Hans de Goede wrote:
> On 07-08-17 17:41, Mark Brown wrote:

> > I2C has a perfectly good platform_data pointer in the board info for
> > this stuff.

> True, so you are suggesting that I define a bq24190_platform_data
> struct with a regulator_init_data pointer in there I guess?

Yes.  

> I don't think the power-supply maintainers will be enthusiastic
> about this (hi Sebastian). But that does make sense and is
> actually a good idea for tackling the problem of regulator_init_data.

Why not?  This is just really standard usage of platform data.

> Would extending the struct regulator_map with a const char *provider_name:

> struct regulator_map {
> struct list_head list;
> const char *dev_name;   /* The dev_name() for the consumer */
> const char *supply;
> struct regulator_dev *regulator;
>   const char *provider;   /* The dev_name() for the regulator parent-dev 
> */
> };

Please don't invent new terminology like this.  Just call it a regulator
name.

> Alternatively the entry could additionally contain a provider_supply_name
> so that we can make arbitrary consumer-dev-name + consumer-supply-name
> provider-dev-name + provider-supply-name matches. That would probably
> be more flexible then requiring the supply name to match.

I'm sorry but I can't follow what you mean here.  What do you mean by
"provider_supply_name"?


signature.asc
Description: PGP signature
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] Staging: comedi: addi_apci_3501: fixed a CamelCase coding style issue

[Ian Abbott]

On 08/08/17 08:42, Chi-Chun Hsu wrote:

Fixed a coding style issue.

Signed-off-by: Chi-Chun Hsu 
---
  drivers/staging/comedi/drivers/addi_apci_3501.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/comedi/drivers/addi_apci_3501.c 
b/drivers/staging/comedi/drivers/addi_apci_3501.c
index 40ff914..b2f6712 100644
--- a/drivers/staging/comedi/drivers/addi_apci_3501.c
+++ b/drivers/staging/comedi/drivers/addi_apci_3501.c
@@ -68,7 +68,7 @@
  struct apci3501_private {
unsigned long amcc;
unsigned long tcw;
-   struct task_struct *tsk_Current;
+   struct task_struct *tsk_current;
unsigned char timer_mode;
  };
  
@@ -273,7 +273,7 @@ static irqreturn_t apci3501_interrupt(int irq, void *d)

}
  
  	/* Enable Interrupt Send a signal to from kernel to user space */

-   send_sig(SIGIO, devpriv->tsk_Current, 0);
+   send_sig(SIGIO, devpriv->tsk_current, 0);
ctrl = inl(devpriv->tcw + ADDI_TCW_CTRL_REG);
ctrl &= ~(ADDI_TCW_CTRL_GATE | ADDI_TCW_CTRL_TRIG |
  ADDI_TCW_CTRL_IRQ_ENA);



This patch is incomplete (the 'tsk_Current' member is also used in 
"drivers/staging/comedi/addi-data/hwdrv_apci3501.c" which is #include'd 
by "addi_apci_3501.c") and out-of-date (the 'tsk_Current' member was 
removed in kernel version 4.9 by commit a6672530f6fc ("staging: comedi: 
addi_apci_3501: remove timer/counter subdevice support").


--
-=( Ian Abbott @ MEV Ltd.E-mail:  )=-
-=(  Web: http://www.mev.co.uk/  )=-
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH] staging: ccree: constify dev_pm_ops structures.

[Arvind Yadav]

dev_pm_ops are not supposed to change at runtime. All functions
working with dev_pm_ops provided by  work with const
dev_pm_ops. So mark the non-const structs as const.

Signed-off-by: Arvind Yadav 
---
 drivers/staging/ccree/ssi_driver.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/ccree/ssi_driver.c 
b/drivers/staging/ccree/ssi_driver.c
index e0faca0..1753906 100644
--- a/drivers/staging/ccree/ssi_driver.c
+++ b/drivers/staging/ccree/ssi_driver.c
@@ -549,7 +549,7 @@ static int cc7x_remove(struct platform_device *plat_dev)
 }
 
 #if defined(CONFIG_PM_RUNTIME) || defined(CONFIG_PM_SLEEP)
-static struct dev_pm_ops arm_cc7x_driver_pm = {
+static const struct dev_pm_ops arm_cc7x_driver_pm = {
SET_RUNTIME_PM_OPS(ssi_power_mgr_runtime_suspend, 
ssi_power_mgr_runtime_resume, NULL)
 };
 #endif
-- 
1.9.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 16/18] power: supply: bq24190_charger: Remove extcon handling

[Hans de Goede]

Hi,

On 08-08-17 10:27, Liam Breck wrote:

Hi Hans,

On Sun, Aug 6, 2017 at 5:35 AM, Hans de Goede  wrote:

Now that drivers/i2c/busses/i2c-cht-wc.c uses
"input-current-limit-from-supplier" instead of "extcon-name" the last
user of the bq24190 extcon code is gone, remove it.

Signed-off-by: Hans de Goede 
---
  drivers/power/supply/bq24190_charger.c | 107 -
  1 file changed, 107 deletions(-)

diff --git a/drivers/power/supply/bq24190_charger.c 
b/drivers/power/supply/bq24190_charger.c
index 1f6424f0772f..0376de6d8e70 100644
--- a/drivers/power/supply/bq24190_charger.c
+++ b/drivers/power/supply/bq24190_charger.c
@@ -11,7 +11,6 @@
  #include 
  #include 
  #include 
-#include 
  #include 
  #include 
  #include 
@@ -155,9 +154,6 @@ struct bq24190_dev_info {
 struct device   *dev;
 struct power_supply *charger;
 struct power_supply *battery;
-   struct extcon_dev   *extcon;
-   struct notifier_block   extcon_nb;
-   struct delayed_work extcon_work;
 charmodel_name[I2C_NAME_SIZE];
 boolinitialized;
 boolirq_event;
@@ -1530,75 +1526,6 @@ static irqreturn_t bq24190_irq_handler_thread(int irq, 
void *data)
 return IRQ_HANDLED;
  }

-static void bq24190_extcon_work(struct work_struct *work)
-{
-   struct bq24190_dev_info *bdi =
-   container_of(work, struct bq24190_dev_info, extcon_work.work);
-   int error, iinlim = 0;
-   u8 v;
-
-   error = pm_runtime_get_sync(bdi->dev);
-   if (error < 0) {
-   dev_warn(bdi->dev, "pm_runtime_get failed: %i\n", error);
-   pm_runtime_put_noidle(bdi->dev);
-   return;
-   }
-
-   if  (extcon_get_state(bdi->extcon, EXTCON_CHG_USB_SDP) == 1)
-   iinlim =  50;
-   else if (extcon_get_state(bdi->extcon, EXTCON_CHG_USB_CDP) == 1 ||
-extcon_get_state(bdi->extcon, EXTCON_CHG_USB_ACA) == 1)
-   iinlim = 150;
-   else if (extcon_get_state(bdi->extcon, EXTCON_CHG_USB_DCP) == 1)
-   iinlim = 200;
-
-   if (iinlim) {
-   error = bq24190_set_field_val(bdi, BQ24190_REG_ISC,
- BQ24190_REG_ISC_IINLIM_MASK,
- BQ24190_REG_ISC_IINLIM_SHIFT,
- bq24190_isc_iinlim_values,
- 
ARRAY_SIZE(bq24190_isc_iinlim_values),
- iinlim);
-   if (error < 0)
-   dev_err(bdi->dev, "Can't set IINLIM: %d\n", error);
-   }
-
-   /* if no charger found and in USB host mode, set OTG 5V boost, else 
normal */
-   if (!iinlim && extcon_get_state(bdi->extcon, EXTCON_USB_HOST) == 1)
-   v = BQ24190_REG_POC_CHG_CONFIG_OTG;
-   else
-   v = BQ24190_REG_POC_CHG_CONFIG_CHARGE;
-
-   error = bq24190_write_mask(bdi, BQ24190_REG_POC,
-  BQ24190_REG_POC_CHG_CONFIG_MASK,
-  BQ24190_REG_POC_CHG_CONFIG_SHIFT,
-  v);
-   if (error < 0)
-   dev_err(bdi->dev, "Can't set CHG_CONFIG: %d\n", error);
-
-   pm_runtime_mark_last_busy(bdi->dev);
-   pm_runtime_put_autosuspend(bdi->dev);
-}
-
-static int bq24190_extcon_event(struct notifier_block *nb, unsigned long event,
-   void *param)
-{
-   struct bq24190_dev_info *bdi =
-   container_of(nb, struct bq24190_dev_info, extcon_nb);
-
-   /*
-* The Power-Good detection may take up to 220ms, sometimes
-* the external charger detection is quicker, and the bq24190 will
-* reset to iinlim based on its own charger detection (which is not
-* hooked up when using external charger detection) resulting in
-* a too low default 500mA iinlim. Delay applying the extcon value
-* for 300ms to avoid this.
-*/
-   queue_delayed_work(system_wq, >extcon_work, msecs_to_jiffies(300));
-
-   return NOTIFY_OK;
-}
-
  static int bq24190_hw_init(struct bq24190_dev_info *bdi)
  {
 u8 v;
@@ -1636,7 +1563,6 @@ static int bq24190_probe(struct i2c_client *client,
 struct device *dev = >dev;
 struct power_supply_config charger_cfg = {}, battery_cfg = {};
 struct bq24190_dev_info *bdi;
-   const char *name;
 int ret;

 if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA)) {
@@ -1668,25 +1594,6 @@ static int bq24190_probe(struct i2c_client *client,
 device_property_read_bool(dev,
   "input-current-limit-from-supplier");

-   /*
-   

Re: [PATCH 15/18] power: supply: bq24190_charger: Get input_current_limit from our supplier

[Hans de Goede]

Hi,

On 08-08-17 10:24, Liam Breck wrote:

Hallo Hans :-)


On Sun, Aug 6, 2017 at 5:35 AM, Hans de Goede  wrote:

On some devices the USB Type-C port power (USB PD 2.0) negotiation is
done by a separate port-controller IC, while the current limit is
controlled through another (charger) IC.

It has been decided to model this by modelling the external Type-C
power brick (adapter/charger) as a power-supply class device which
supplies the charger-IC, with its voltage-now and current-max representing
the negotiated voltage and max current draw.

This commit adds support for this to the bq24190_charger driver by calling
power_supply_set_input_current_limit_from_supplier helper if the
"input-current-limit-from-supplier" device-property is set.

Note this replaces the functionality to get the current-limit from an
extcon device, which will be removed in a follow-up commit.

Signed-off-by: Hans de Goede 
---
  drivers/power/supply/bq24190_charger.c | 14 ++
  1 file changed, 14 insertions(+)

diff --git a/drivers/power/supply/bq24190_charger.c 
b/drivers/power/supply/bq24190_charger.c
index d78e2c6dc127..1f6424f0772f 100644
--- a/drivers/power/supply/bq24190_charger.c
+++ b/drivers/power/supply/bq24190_charger.c
@@ -161,6 +161,7 @@ struct bq24190_dev_info {
 charmodel_name[I2C_NAME_SIZE];
 boolinitialized;
 boolirq_event;
+   boolinput_current_limit_from_supplier;
 struct mutexf_reg_lock;
 u8  f_reg;
 u8  ss_reg;
@@ -1137,6 +1138,14 @@ static int bq24190_charger_property_is_writeable(struct 
power_supply *psy,
 return ret;
  }

+static void bq24190_charger_external_power_changed(struct power_supply *psy)
+{
+   struct bq24190_dev_info *bdi = power_supply_get_drvdata(psy);
+
+   if (bdi->input_current_limit_from_supplier)
+   power_supply_set_input_current_limit_from_supplier(psy);
+}
+
  static enum power_supply_property bq24190_charger_properties[] = {
 POWER_SUPPLY_PROP_CHARGE_TYPE,
 POWER_SUPPLY_PROP_HEALTH,
@@ -1165,6 +1174,7 @@ static const struct power_supply_desc 
bq24190_charger_desc = {
 .get_property   = bq24190_charger_get_property,
 .set_property   = bq24190_charger_set_property,
 .property_is_writeable  = bq24190_charger_property_is_writeable,
+   .external_power_changed = bq24190_charger_external_power_changed,
  };

  /* Battery power supply property routines */
@@ -1654,6 +1664,10 @@ static int bq24190_probe(struct i2c_client *client,
 return -EINVAL;
 }

+   bdi->input_current_limit_from_supplier =
+   device_property_read_bool(dev,
+ "input-current-limit-from-supplier");


Since this invokes the new power_supply_class function, should this be
a devicetree property, not just a driver-to-driver switch? If so, the
property name should probably be defined in
Doc...bindings/power/supply/power_supply.txt.


Well this is a kernel internal thing, specifying a supplier through devicetree
should already be documented, this tells a driver to set its input-current-limit
based on the max-current of its supplier.

So we could documented it, but I think it should probably be prefixed with
"linux," then.

Anyways first lets see what Sebastian thinks of this approach if he nacks
it we don't have to worry about documenting it either :)

Regards,

Hans
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 13/18] power: supply: bq24190_charger: Export 5V boost converter as regulator

[Hans de Goede]

Hi,

On 08-08-17 10:39, Liam Breck wrote:

Hi Hans,

On Sun, Aug 6, 2017 at 5:35 AM, Hans de Goede  wrote:

Register the 5V boost converter as a regulator named
"regulator-bq24190-usb-vbus". Note the name includes "bq24190" because
the bq24190 family is also used on ACPI devices where there are no
device-tree phandles, so regulator_get will fallback to the name and thus
it must be unique on the system.


What we're enabling here is 5V boost for otg host mode, not vbus
generally, so maybe the name should indicate that...

regulator-bq24190-usb-5volt
regulator-bq24190-usb-host
regulator-bq24190-usb-otg-5v


I picked vbus because that gets used a lot already in similar cases,
but I agree that we should probably come up with a better name.

I like "regulator-bq24190-usb-otg-5v", shall I use that for v2?

Regards,

Hans

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 13/18] power: supply: bq24190_charger: Export 5V boost converter as regulator

[Liam Breck]

Hi Hans,

On Sun, Aug 6, 2017 at 5:35 AM, Hans de Goede  wrote:
> Register the 5V boost converter as a regulator named
> "regulator-bq24190-usb-vbus". Note the name includes "bq24190" because
> the bq24190 family is also used on ACPI devices where there are no
> device-tree phandles, so regulator_get will fallback to the name and thus
> it must be unique on the system.

What we're enabling here is 5V boost for otg host mode, not vbus
generally, so maybe the name should indicate that...

regulator-bq24190-usb-5volt
regulator-bq24190-usb-host
regulator-bq24190-usb-otg-5v

Tony, thoughts?
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 16/18] power: supply: bq24190_charger: Remove extcon handling

[Liam Breck]

Hi Hans,

On Sun, Aug 6, 2017 at 5:35 AM, Hans de Goede  wrote:
> Now that drivers/i2c/busses/i2c-cht-wc.c uses
> "input-current-limit-from-supplier" instead of "extcon-name" the last
> user of the bq24190 extcon code is gone, remove it.
>
> Signed-off-by: Hans de Goede 
> ---
>  drivers/power/supply/bq24190_charger.c | 107 
> -
>  1 file changed, 107 deletions(-)
>
> diff --git a/drivers/power/supply/bq24190_charger.c 
> b/drivers/power/supply/bq24190_charger.c
> index 1f6424f0772f..0376de6d8e70 100644
> --- a/drivers/power/supply/bq24190_charger.c
> +++ b/drivers/power/supply/bq24190_charger.c
> @@ -11,7 +11,6 @@
>  #include 
>  #include 
>  #include 
> -#include 
>  #include 
>  #include 
>  #include 
> @@ -155,9 +154,6 @@ struct bq24190_dev_info {
> struct device   *dev;
> struct power_supply *charger;
> struct power_supply *battery;
> -   struct extcon_dev   *extcon;
> -   struct notifier_block   extcon_nb;
> -   struct delayed_work extcon_work;
> charmodel_name[I2C_NAME_SIZE];
> boolinitialized;
> boolirq_event;
> @@ -1530,75 +1526,6 @@ static irqreturn_t bq24190_irq_handler_thread(int irq, 
> void *data)
> return IRQ_HANDLED;
>  }
>
> -static void bq24190_extcon_work(struct work_struct *work)
> -{
> -   struct bq24190_dev_info *bdi =
> -   container_of(work, struct bq24190_dev_info, extcon_work.work);
> -   int error, iinlim = 0;
> -   u8 v;
> -
> -   error = pm_runtime_get_sync(bdi->dev);
> -   if (error < 0) {
> -   dev_warn(bdi->dev, "pm_runtime_get failed: %i\n", error);
> -   pm_runtime_put_noidle(bdi->dev);
> -   return;
> -   }
> -
> -   if  (extcon_get_state(bdi->extcon, EXTCON_CHG_USB_SDP) == 1)
> -   iinlim =  50;
> -   else if (extcon_get_state(bdi->extcon, EXTCON_CHG_USB_CDP) == 1 ||
> -extcon_get_state(bdi->extcon, EXTCON_CHG_USB_ACA) == 1)
> -   iinlim = 150;
> -   else if (extcon_get_state(bdi->extcon, EXTCON_CHG_USB_DCP) == 1)
> -   iinlim = 200;
> -
> -   if (iinlim) {
> -   error = bq24190_set_field_val(bdi, BQ24190_REG_ISC,
> - BQ24190_REG_ISC_IINLIM_MASK,
> - BQ24190_REG_ISC_IINLIM_SHIFT,
> - bq24190_isc_iinlim_values,
> - 
> ARRAY_SIZE(bq24190_isc_iinlim_values),
> - iinlim);
> -   if (error < 0)
> -   dev_err(bdi->dev, "Can't set IINLIM: %d\n", error);
> -   }
> -
> -   /* if no charger found and in USB host mode, set OTG 5V boost, else 
> normal */
> -   if (!iinlim && extcon_get_state(bdi->extcon, EXTCON_USB_HOST) == 1)
> -   v = BQ24190_REG_POC_CHG_CONFIG_OTG;
> -   else
> -   v = BQ24190_REG_POC_CHG_CONFIG_CHARGE;
> -
> -   error = bq24190_write_mask(bdi, BQ24190_REG_POC,
> -  BQ24190_REG_POC_CHG_CONFIG_MASK,
> -  BQ24190_REG_POC_CHG_CONFIG_SHIFT,
> -  v);
> -   if (error < 0)
> -   dev_err(bdi->dev, "Can't set CHG_CONFIG: %d\n", error);
> -
> -   pm_runtime_mark_last_busy(bdi->dev);
> -   pm_runtime_put_autosuspend(bdi->dev);
> -}
> -
> -static int bq24190_extcon_event(struct notifier_block *nb, unsigned long 
> event,
> -   void *param)
> -{
> -   struct bq24190_dev_info *bdi =
> -   container_of(nb, struct bq24190_dev_info, extcon_nb);
> -
> -   /*
> -* The Power-Good detection may take up to 220ms, sometimes
> -* the external charger detection is quicker, and the bq24190 will
> -* reset to iinlim based on its own charger detection (which is not
> -* hooked up when using external charger detection) resulting in
> -* a too low default 500mA iinlim. Delay applying the extcon value
> -* for 300ms to avoid this.
> -*/
> -   queue_delayed_work(system_wq, >extcon_work, 
> msecs_to_jiffies(300));
> -
> -   return NOTIFY_OK;
> -}
> -
>  static int bq24190_hw_init(struct bq24190_dev_info *bdi)
>  {
> u8 v;
> @@ -1636,7 +1563,6 @@ static int bq24190_probe(struct i2c_client *client,
> struct device *dev = >dev;
> struct power_supply_config charger_cfg = {}, battery_cfg = {};
> struct bq24190_dev_info *bdi;
> -   const char *name;
> int ret;
>
> if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA)) {
> @@ -1668,25 +1594,6 @@ static int 

Re: [PATCH 15/18] power: supply: bq24190_charger: Get input_current_limit from our supplier

[Liam Breck]

Hallo Hans :-)


On Sun, Aug 6, 2017 at 5:35 AM, Hans de Goede  wrote:
> On some devices the USB Type-C port power (USB PD 2.0) negotiation is
> done by a separate port-controller IC, while the current limit is
> controlled through another (charger) IC.
>
> It has been decided to model this by modelling the external Type-C
> power brick (adapter/charger) as a power-supply class device which
> supplies the charger-IC, with its voltage-now and current-max representing
> the negotiated voltage and max current draw.
>
> This commit adds support for this to the bq24190_charger driver by calling
> power_supply_set_input_current_limit_from_supplier helper if the
> "input-current-limit-from-supplier" device-property is set.
>
> Note this replaces the functionality to get the current-limit from an
> extcon device, which will be removed in a follow-up commit.
>
> Signed-off-by: Hans de Goede 
> ---
>  drivers/power/supply/bq24190_charger.c | 14 ++
>  1 file changed, 14 insertions(+)
>
> diff --git a/drivers/power/supply/bq24190_charger.c 
> b/drivers/power/supply/bq24190_charger.c
> index d78e2c6dc127..1f6424f0772f 100644
> --- a/drivers/power/supply/bq24190_charger.c
> +++ b/drivers/power/supply/bq24190_charger.c
> @@ -161,6 +161,7 @@ struct bq24190_dev_info {
> charmodel_name[I2C_NAME_SIZE];
> boolinitialized;
> boolirq_event;
> +   boolinput_current_limit_from_supplier;
> struct mutexf_reg_lock;
> u8  f_reg;
> u8  ss_reg;
> @@ -1137,6 +1138,14 @@ static int 
> bq24190_charger_property_is_writeable(struct power_supply *psy,
> return ret;
>  }
>
> +static void bq24190_charger_external_power_changed(struct power_supply *psy)
> +{
> +   struct bq24190_dev_info *bdi = power_supply_get_drvdata(psy);
> +
> +   if (bdi->input_current_limit_from_supplier)
> +   power_supply_set_input_current_limit_from_supplier(psy);
> +}
> +
>  static enum power_supply_property bq24190_charger_properties[] = {
> POWER_SUPPLY_PROP_CHARGE_TYPE,
> POWER_SUPPLY_PROP_HEALTH,
> @@ -1165,6 +1174,7 @@ static const struct power_supply_desc 
> bq24190_charger_desc = {
> .get_property   = bq24190_charger_get_property,
> .set_property   = bq24190_charger_set_property,
> .property_is_writeable  = bq24190_charger_property_is_writeable,
> +   .external_power_changed = bq24190_charger_external_power_changed,
>  };
>
>  /* Battery power supply property routines */
> @@ -1654,6 +1664,10 @@ static int bq24190_probe(struct i2c_client *client,
> return -EINVAL;
> }
>
> +   bdi->input_current_limit_from_supplier =
> +   device_property_read_bool(dev,
> + 
> "input-current-limit-from-supplier");

Since this invokes the new power_supply_class function, should this be
a devicetree property, not just a driver-to-driver switch? If so, the
property name should probably be defined in
Doc...bindings/power/supply/power_supply.txt.

My latest bq24190 series has a new DT doc, which should ref a new DT property.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel