Re: [PATCH v7 00/17] wilc1000: move out of staging

2020-06-24 Thread Kalle Valo
 writes:

> From: Ajay Singh 
>
> This patch series is to review and move wilc1000 driver out of staging.
> Most of the review comments received in [1] & [2] are addressed in the
> latest code.
> Please review and provide your inputs.
>
> [1]. 
> https://lore.kernel.org/linux-wireless/1537957525-11467-1-git-send-email-ajay.kat...@microchip.com/
> [2]. 
> https://lore.kernel.org/linux-wireless/1562896697-8002-1-git-send-email-ajay.kat...@microchip.com/
>
> Changes since v6:
>  - added Reviewed-by tag received for DT binding document patch earlier.
>* https://lore.kernel.org/linux-wireless/20200405013235.GA24105@bogus
>  - merged latest driver and included --base commit as suggested.

Greg, in preparation for moving the driver to drivers/net/wireless can I
ask you to not to take wilc1000 patches for the time being? I think that
way it would be easier to move the driver between trees if there are no
changes after v5.8-rc1. Or is there a better way handle the move?

I have not reviewed the latest version yet but I'm hoping it's ready
now. I would also appreciate comments from other people about the
readiness of this driver.

-- 
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH v7 06/17] wilc1000: add cfg80211.c

2020-06-24 Thread Johannes Berg
On Tue, 2020-06-23 at 11:00 +, ajay.kat...@microchip.com wrote:
> 
> +struct wilc_p2p_pub_act_frame {
> + u8 category;
> + u8 action;
> + u8 oui[3];
> + u8 oui_type;
> + u8 oui_subtype;
> + u8 dialog_token;
> + u8 elem[];
> +} __packed;
> +
> +struct wilc_vendor_specific_ie {
> + u8 tag_number;
> + u8 tag_len;
> + u8 oui[3];
> + u8 oui_type;
> + u8 attr[];
> +} __packed;
> +
> +struct wilc_attr_entry {
> + u8  attr_type;
> + __le16 attr_len;
> + u8 val[];
> +} __packed;
> +
> +struct wilc_attr_oper_ch {
> + u8 attr_type;
> + __le16 attr_len;
> + u8 country_code[IEEE80211_COUNTRY_STRING_LEN];
> + u8 op_class;
> + u8 op_channel;
> +} __packed;
> +
> +struct wilc_attr_ch_list {
> + u8 attr_type;
> + __le16 attr_len;
> + u8 country_code[IEEE80211_COUNTRY_STRING_LEN];
> + u8 elem[];
> +} __packed;
> +
> +struct wilc_ch_list_elem {
> + u8 op_class;
> + u8 no_of_channels;
> + u8 ch_list[];
> +} __packed;

It seems like these should be used from ieee80211.h, and/or added there
if they don't already exist?

> +static int wilc_wfi_cfg_copy_wpa_info(struct wilc_wfi_key *key_info,
> +   struct key_params *params)
> +{
> + kfree(key_info->key);
> +
> + key_info->key = kmemdup(params->key, params->key_len, GFP_KERNEL);
> + if (!key_info->key)
> + return -ENOMEM;
> +
> + kfree(key_info->seq);
> +
> + if (params->seq_len > 0) {
> + key_info->seq = kmemdup(params->seq, params->seq_len,
> + GFP_KERNEL);
> + if (!key_info->seq)
> + return -ENOMEM;

you may leak key_info->key here?

> +static inline void wilc_wfi_cfg_parse_ch_attr(u8 *buf, u32 len, u8 sta_ch)

That's a bit big to be forced inline, imho. if it's used only once the
compiler will inline it anyway.

> + d = (struct wilc_p2p_pub_act_frame *)(&mgmt->u.action);
> + if (d->oui_subtype != GO_NEG_REQ && d->oui_subtype != GO_NEG_RSP &&
> + d->oui_subtype != P2P_INV_REQ && d->oui_subtype != P2P_INV_RSP)
> + goto out_rx_mgmt;
> +
> + vendor_ie = cfg80211_find_vendor_ie(WLAN_OUI_WFA, WLAN_OUI_TYPE_WFA_P2P,
> + buff + ie_offset, size - ie_offset);
> + if (!vendor_ie)
> + goto out_rx_mgmt;
> +
> + p = (struct wilc_vendor_specific_ie *)vendor_ie;
> + wilc_wfi_cfg_parse_ch_attr(p->attr, p->tag_len - 4, vif->wilc->sta_ch);

but overall, why do you even need this? I don't think this is normally
handled in the driver, but wpa_s?


Anyway, I'm not convinced that we should really keep kicking this back
over minor issues like this ... better to merge it and fix later, imho.

johannes


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


Re: [PATCH v7 00/17] wilc1000: move out of staging

2020-06-24 Thread Greg KH
On Wed, Jun 24, 2020 at 11:50:07AM +0300, Kalle Valo wrote:
>  writes:
> 
> > From: Ajay Singh 
> >
> > This patch series is to review and move wilc1000 driver out of staging.
> > Most of the review comments received in [1] & [2] are addressed in the
> > latest code.
> > Please review and provide your inputs.
> >
> > [1]. 
> > https://lore.kernel.org/linux-wireless/1537957525-11467-1-git-send-email-ajay.kat...@microchip.com/
> > [2]. 
> > https://lore.kernel.org/linux-wireless/1562896697-8002-1-git-send-email-ajay.kat...@microchip.com/
> >
> > Changes since v6:
> >  - added Reviewed-by tag received for DT binding document patch earlier.
> >* https://lore.kernel.org/linux-wireless/20200405013235.GA24105@bogus
> >  - merged latest driver and included --base commit as suggested.
> 
> Greg, in preparation for moving the driver to drivers/net/wireless can I
> ask you to not to take wilc1000 patches for the time being? I think that
> way it would be easier to move the driver between trees if there are no
> changes after v5.8-rc1. Or is there a better way handle the move?

The best way is for there to be a series of patches that just adds the
driver to the "real" part of the tree, and when that is merged, let me
know and I will just delete the driver version in the staging tree.

Does that work for you?

thanks,

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


Re: [PATCH v7 17/17] wilc1000: add Makefile and Kconfig files for wilc1000 compilation

2020-06-24 Thread Ajay.Kathat



On 23/06/20 8:22 pm, kernel test robot wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the 
> content is safe
> 
> Hi,
> 
> I love your patch! Perhaps something to improve:
> 
> [auto build test WARNING on a15a20acc980342c97d804c5fae1cfc0cd7712a9]
> 
> url:
> https://github.com/0day-ci/linux/commits/Ajay-Kathat-microchip-com/wilc1000-move-out-of-staging/20200623-190333
> base:a15a20acc980342c97d804c5fae1cfc0cd7712a9
> config: ia64-allmodconfig (attached as .config)
> compiler: ia64-linux-gcc (GCC) 9.3.0
> reproduce (this is a W=1 build):
> wget 
> https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
> ~/bin/make.cross
> chmod +x ~/bin/make.cross
> # save the attached .config to linux build tree
> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross 
> ARCH=ia64
> 
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot 
> 
> All warnings (new ones prefixed by >>):
> 
>drivers/net/wireless/microchip/wilc1000/mon.c: In function 
> 'wilc_wfi_init_mon_interface':
>>> drivers/net/wireless/microchip/wilc1000/mon.c:232:2: warning: 'strncpy' 
>>> specified bound 16 equals destination size [-Wstringop-truncation]
>  232 |  strncpy(wl->monitor_dev->name, name, IFNAMSIZ);
>  |  ^~

Though replacing 'strncpy' with 'strlcpy' will help to suppress this
warning but not sure if its true alarm because next line sets NULL
termination for string.
I agree 'strlcpy' is better candidate here and reduces one extra line.

> 
> vim +/strncpy +232 drivers/net/wireless/microchip/wilc1000/mon.c
> 
> daf8b5f14a7066 Ajay Singh 2020-06-23  216
> daf8b5f14a7066 Ajay Singh 2020-06-23  217  struct net_device 
> *wilc_wfi_init_mon_interface(struct wilc *wl,
> daf8b5f14a7066 Ajay Singh 2020-06-23  218 
>  const char *name,
> daf8b5f14a7066 Ajay Singh 2020-06-23  219 
>  struct net_device *real_dev)
> daf8b5f14a7066 Ajay Singh 2020-06-23  220  {
> daf8b5f14a7066 Ajay Singh 2020-06-23  221   struct wilc_wfi_mon_priv 
> *priv;
> daf8b5f14a7066 Ajay Singh 2020-06-23  222
> daf8b5f14a7066 Ajay Singh 2020-06-23  223   /* If monitor interface is 
> already initialized, return it */
> daf8b5f14a7066 Ajay Singh 2020-06-23  224   if (wl->monitor_dev)
> daf8b5f14a7066 Ajay Singh 2020-06-23  225   return 
> wl->monitor_dev;
> daf8b5f14a7066 Ajay Singh 2020-06-23  226
> daf8b5f14a7066 Ajay Singh 2020-06-23  227   wl->monitor_dev = 
> alloc_etherdev(sizeof(struct wilc_wfi_mon_priv));
> daf8b5f14a7066 Ajay Singh 2020-06-23  228   if (!wl->monitor_dev)
> daf8b5f14a7066 Ajay Singh 2020-06-23  229   return NULL;
> daf8b5f14a7066 Ajay Singh 2020-06-23  230
> daf8b5f14a7066 Ajay Singh 2020-06-23  231   wl->monitor_dev->type = 
> ARPHRD_IEEE80211_RADIOTAP;
> daf8b5f14a7066 Ajay Singh 2020-06-23 @232   
> strncpy(wl->monitor_dev->name, name, IFNAMSIZ);
> daf8b5f14a7066 Ajay Singh 2020-06-23  233   
> wl->monitor_dev->name[IFNAMSIZ - 1] = 0;
> daf8b5f14a7066 Ajay Singh 2020-06-23  234   wl->monitor_dev->netdev_ops = 
> &wilc_wfi_netdev_ops;
> daf8b5f14a7066 Ajay Singh 2020-06-23  235   
> wl->monitor_dev->needs_free_netdev = true;
> daf8b5f14a7066 Ajay Singh 2020-06-23  236
> daf8b5f14a7066 Ajay Singh 2020-06-23  237   if 
> (register_netdevice(wl->monitor_dev)) {
> daf8b5f14a7066 Ajay Singh 2020-06-23  238   netdev_err(real_dev, 
> "register_netdevice failed\n");
> daf8b5f14a7066 Ajay Singh 2020-06-23  239   return NULL;
> daf8b5f14a7066 Ajay Singh 2020-06-23  240   }
> daf8b5f14a7066 Ajay Singh 2020-06-23  241   priv = 
> netdev_priv(wl->monitor_dev);
> daf8b5f14a7066 Ajay Singh 2020-06-23  242   if (!priv)
> daf8b5f14a7066 Ajay Singh 2020-06-23  243   return NULL;
> daf8b5f14a7066 Ajay Singh 2020-06-23  244
> daf8b5f14a7066 Ajay Singh 2020-06-23  245   priv->real_ndev = real_dev;
> daf8b5f14a7066 Ajay Singh 2020-06-23  246
> daf8b5f14a7066 Ajay Singh 2020-06-23  247   return wl->monitor_dev;
> daf8b5f14a7066 Ajay Singh 2020-06-23  248  }
> daf8b5f14a7066 Ajay Singh 2020-06-23  249
> 
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org
> 
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH v7 00/17] wilc1000: move out of staging

2020-06-24 Thread Kalle Valo
Greg KH  writes:

> On Wed, Jun 24, 2020 at 11:50:07AM +0300, Kalle Valo wrote:
>>  writes:
>> 
>> > From: Ajay Singh 
>> >
>> > This patch series is to review and move wilc1000 driver out of staging.
>> > Most of the review comments received in [1] & [2] are addressed in the
>> > latest code.
>> > Please review and provide your inputs.
>> >
>> > [1]. 
>> > https://lore.kernel.org/linux-wireless/1537957525-11467-1-git-send-email-ajay.kat...@microchip.com/
>> > [2]. 
>> > https://lore.kernel.org/linux-wireless/1562896697-8002-1-git-send-email-ajay.kat...@microchip.com/
>> >
>> > Changes since v6:
>> >  - added Reviewed-by tag received for DT binding document patch earlier.
>> >* https://lore.kernel.org/linux-wireless/20200405013235.GA24105@bogus
>> >  - merged latest driver and included --base commit as suggested.
>> 
>> Greg, in preparation for moving the driver to drivers/net/wireless can I
>> ask you to not to take wilc1000 patches for the time being? I think that
>> way it would be easier to move the driver between trees if there are no
>> changes after v5.8-rc1. Or is there a better way handle the move?
>
> The best way is for there to be a series of patches that just adds the
> driver to the "real" part of the tree, and when that is merged, let me
> know and I will just delete the driver version in the staging tree.
>
> Does that work for you?

It would be fine for me but won't that approach break the build (eg.
allyesconfig) due to two duplicate versions of the same driver in
wireless-drivers-next?

What I was thinking that Ajay would create a patch moving the driver
from drivers/staging/wilc1000 to
drivers/net/wireless/microchip/wilc1000. Using 'git mv' and 'git
format-patch --find-renames' the patch should be really small, mostly
just renames and small changes to Kconfig, Makefile and MAINTAINERS
files. But this of course would require that there are no wilc1000
patches in your tree until you get the driver move commit during the
next merge window, otherwise we would see conflicts between staging-next
and wireless-drivers-next.

But I don't have any strong opinions, whatever is easiest for everyone :)

For reference wireless-drivers-next is merged like this:

wireless-drivers-next -> net-next -> linus

And naturally I would be aiming this for the v5.9 merge window.

-- 
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH 03/50] staging: mmal-vchiq: Avoid use of bool in structures

2020-06-24 Thread Nicolas Saenz Julienne
On Tue, 2020-06-23 at 15:11 -0700, Joe Perches wrote:
> On Tue, 2020-06-23 at 18:41 +0200, Nicolas Saenz Julienne wrote:
> > From: Dave Stevenson 
> > 
> > Fixes up a checkpatch error "Avoid using bool structure members
> > because of possible alignment issues".
> []
> > diff --git a/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c
> > b/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c
> []
> > @@ -1754,7 +1754,7 @@ int vchiq_mmal_component_enable(struct
> > vchiq_mmal_instance *instance,
> >  
> > ret = enable_component(instance, component);
> > if (ret == 0)
> > -   component->enabled = true;
> > +   component->enabled = 1;
> 
> This change does not match the commit description.
> 
> Also, checkpatch does not emit a warning here.

Fair enough I'll drop it for v2.

Regards,
Nicolas



signature.asc
Description: This is a digitally signed message part
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH] staging: media: soc_camera: Enclose macro with parentheses

2020-06-24 Thread Hans Verkuil
On 24/04/2020 21:56, Guilherme Ricioli wrote:
> Fix checkpatch error "ERROR: Macros with complex values should be
> enclosed in parentheses" in soc_camera.c:241.
> 
> Signed-off-by: Guilherme Ricioli 
> ---
>  drivers/staging/media/soc_camera/soc_camera.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/media/soc_camera/soc_camera.c 
> b/drivers/staging/media/soc_camera/soc_camera.c
> index 39f513f..c2f96ea 100644
> --- a/drivers/staging/media/soc_camera/soc_camera.c
> +++ b/drivers/staging/media/soc_camera/soc_camera.c
> @@ -238,8 +238,8 @@ unsigned long soc_camera_apply_board_flags(struct 
> soc_camera_subdev_desc *ssdd,
>  }
>  EXPORT_SYMBOL(soc_camera_apply_board_flags);
>  
> -#define pixfmtstr(x) (x) & 0xff, ((x) >> 8) & 0xff, ((x) >> 16) & 0xff, \
> - ((x) >> 24) & 0xff
> +#define pixfmtstr(x) ((x) & 0xff, ((x) >> 8) & 0xff, ((x) >> 16) & 0xff, \
> + ((x) >> 24) & 0xff)
>  
>  static int soc_camera_try_fmt(struct soc_camera_device *icd,
> struct v4l2_format *f)
> 

Thank you for the patch, but this driver is deprecated and in fact depends on 
BROKEN
(so it is never build).

Because of that I'll drop this patch.

Regards,

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


Re: [PATCH 15/50] staging: vchi: Unify struct shim_service and struct vchi_service_handle

2020-06-24 Thread Dan Carpenter
On Tue, Jun 23, 2020 at 06:42:01PM +0200, Nicolas Saenz Julienne wrote:
> @@ -437,12 +432,10 @@ static void service_free(struct shim_service *service)
>  
>  int32_t vchi_service_open(struct vchiq_instance *instance,
>   struct service_creation *setup,
> - struct vchi_service_handle **handle)
> + struct vchi_service **service)
>  {
> - struct shim_service *service = service_alloc(instance, setup);
> -
> - *handle = (struct vchi_service_handle *)service;
>  
> + *service = service_alloc(instance, setup);
>   if (service) {

This should be checking "*service".

>   struct vchiq_service_params params;
>   enum vchiq_status status;
> @@ -450,27 +443,25 @@ int32_t vchi_service_open(struct vchiq_instance 
> *instance,
>   memset(¶ms, 0, sizeof(params));
>   params.fourcc = setup->service_id;
>   params.callback = shim_callback;
> - params.userdata = service;
> + params.userdata = *service;
>   params.version = setup->version.version;
>   params.version_min = setup->version.version_min;
>  
>   status = vchiq_open_service(instance, ¶ms,
> - &service->handle);
> + &((*service)->handle));
>   if (status != VCHIQ_SUCCESS) {
> - service_free(service);
> - service = NULL;
> - *handle = NULL;
> + service_free(*service);
> + *service = NULL;
>   }
>   }
>  
> - return service ? 0 : -1;
> + return *service ? 0 : -1;
>  }
>  EXPORT_SYMBOL(vchi_service_open);
>  

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


Re: [PATCH 15/50] staging: vchi: Unify struct shim_service and struct vchi_service_handle

2020-06-24 Thread Nicolas Saenz Julienne
On Wed, 2020-06-24 at 17:11 +0300, Dan Carpenter wrote:
> On Tue, Jun 23, 2020 at 06:42:01PM +0200, Nicolas Saenz Julienne wrote:
> > @@ -437,12 +432,10 @@ static void service_free(struct shim_service *service)
> >  
> >  int32_t vchi_service_open(struct vchiq_instance *instance,
> > struct service_creation *setup,
> > -   struct vchi_service_handle **handle)
> > +   struct vchi_service **service)
> >  {
> > -   struct shim_service *service = service_alloc(instance, setup);
> > -
> > -   *handle = (struct vchi_service_handle *)service;
> >  
> > +   *service = service_alloc(instance, setup);
> > if (service) {
> 
> This should be checking "*service".
> 

Of course, thanks!

Reards,
Nicolas




signature.asc
Description: This is a digitally signed message part
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH v7 00/17] wilc1000: move out of staging

2020-06-24 Thread Greg KH
On Wed, Jun 24, 2020 at 12:49:24PM +0300, Kalle Valo wrote:
> Greg KH  writes:
> 
> > On Wed, Jun 24, 2020 at 11:50:07AM +0300, Kalle Valo wrote:
> >>  writes:
> >> 
> >> > From: Ajay Singh 
> >> >
> >> > This patch series is to review and move wilc1000 driver out of staging.
> >> > Most of the review comments received in [1] & [2] are addressed in the
> >> > latest code.
> >> > Please review and provide your inputs.
> >> >
> >> > [1]. 
> >> > https://lore.kernel.org/linux-wireless/1537957525-11467-1-git-send-email-ajay.kat...@microchip.com/
> >> > [2]. 
> >> > https://lore.kernel.org/linux-wireless/1562896697-8002-1-git-send-email-ajay.kat...@microchip.com/
> >> >
> >> > Changes since v6:
> >> >  - added Reviewed-by tag received for DT binding document patch earlier.
> >> >* https://lore.kernel.org/linux-wireless/20200405013235.GA24105@bogus
> >> >  - merged latest driver and included --base commit as suggested.
> >> 
> >> Greg, in preparation for moving the driver to drivers/net/wireless can I
> >> ask you to not to take wilc1000 patches for the time being? I think that
> >> way it would be easier to move the driver between trees if there are no
> >> changes after v5.8-rc1. Or is there a better way handle the move?
> >
> > The best way is for there to be a series of patches that just adds the
> > driver to the "real" part of the tree, and when that is merged, let me
> > know and I will just delete the driver version in the staging tree.
> >
> > Does that work for you?
> 
> It would be fine for me but won't that approach break the build (eg.
> allyesconfig) due to two duplicate versions of the same driver in
> wireless-drivers-next?

For maybe one day, yes, but that's all.

> What I was thinking that Ajay would create a patch moving the driver
> from drivers/staging/wilc1000 to
> drivers/net/wireless/microchip/wilc1000. Using 'git mv' and 'git
> format-patch --find-renames' the patch should be really small, mostly
> just renames and small changes to Kconfig, Makefile and MAINTAINERS
> files. But this of course would require that there are no wilc1000
> patches in your tree until you get the driver move commit during the
> next merge window, otherwise we would see conflicts between staging-next
> and wireless-drivers-next.
> 
> But I don't have any strong opinions, whatever is easiest for everyone :)

It's kind of hard to review patches that do moves, but if you all want
to do that, that's fine with me.

Note, I can't guarantee that I'll not take any wilc1000 patches, I'll
probably forget, but git mv will handle all of that just fine.

thanks,

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


[PATCH v2 2/4] media: staging: tegra-vde: Runtime PM is always available on Tegra

2020-06-24 Thread Dmitry Osipenko
Runtime PM is always available on Tegra nowadays since commit 40b2bb1b132a
("ARM: tegra: enforce PM requirement"), hence the case of unavailable RPM
doesn't need to be handled.

Signed-off-by: Dmitry Osipenko 
---
 drivers/staging/media/tegra-vde/vde.c | 16 
 1 file changed, 16 deletions(-)

diff --git a/drivers/staging/media/tegra-vde/vde.c 
b/drivers/staging/media/tegra-vde/vde.c
index 803e5dda4bb5..85cbbc8f70d3 100644
--- a/drivers/staging/media/tegra-vde/vde.c
+++ b/drivers/staging/media/tegra-vde/vde.c
@@ -1068,17 +1068,8 @@ static int tegra_vde_probe(struct platform_device *pdev)
pm_runtime_use_autosuspend(dev);
pm_runtime_set_autosuspend_delay(dev, 300);
 
-   if (!pm_runtime_enabled(dev)) {
-   err = tegra_vde_runtime_resume(dev);
-   if (err)
-   goto err_misc_unreg;
-   }
-
return 0;
 
-err_misc_unreg:
-   misc_deregister(&vde->miscdev);
-
 err_deinit_iommu:
tegra_vde_iommu_deinit(vde);
 
@@ -1093,13 +1084,6 @@ static int tegra_vde_remove(struct platform_device *pdev)
 {
struct tegra_vde *vde = platform_get_drvdata(pdev);
struct device *dev = &pdev->dev;
-   int err;
-
-   if (!pm_runtime_enabled(dev)) {
-   err = tegra_vde_runtime_suspend(dev);
-   if (err)
-   return err;
-   }
 
pm_runtime_dont_use_autosuspend(dev);
pm_runtime_disable(dev);
-- 
2.26.0

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


[PATCH v2 1/4] media: staging: tegra-vde: Balance runtime PM use-count on resume failure

2020-06-24 Thread Dmitry Osipenko
The RPM's use-count is getting incremented regardless of the
pm_runtime_get_sync() success or fail. It's up to a driver how to
handle the failed RPM. In the case of VDE driver, the RPM's use-count
should be restored if runtime-resume fails. Use pm_runtime_put_noidle(),
which is the most straight-forward variant to balance the RPM, confirmed
by Rafael J. Wysocki.

Link: 
https://lore.kernel.org/linux-i2c/cajz5v0i87ngcy9+kxubscdpdybyr8ypqwcggbfn+v-wdd69...@mail.gmail.com/
Signed-off-by: Dmitry Osipenko 
---
 drivers/staging/media/tegra-vde/vde.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/media/tegra-vde/vde.c 
b/drivers/staging/media/tegra-vde/vde.c
index d3e63512a765..803e5dda4bb5 100644
--- a/drivers/staging/media/tegra-vde/vde.c
+++ b/drivers/staging/media/tegra-vde/vde.c
@@ -776,8 +776,10 @@ static int tegra_vde_ioctl_decode_h264(struct tegra_vde 
*vde,
goto release_dpb_frames;
 
ret = pm_runtime_get_sync(dev);
-   if (ret < 0)
+   if (ret < 0) {
+   pm_runtime_put_noidle(dev);
goto unlock;
+   }
 
/*
 * We rely on the VDE registers reset value, otherwise VDE
-- 
2.26.0

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


[PATCH v2 0/4] Tegra Video Decoder driver power management corrections

2020-06-24 Thread Dmitry Osipenko
Hello,

This small series addresses a Runtime PM issue that was discovered during
of Tegra VI driver reviewing by balancing RPM usage count on RPM resume
failure. Secondly it fixes reboot on some Tegra devices due to bootloader
expecting VDE power partition to be ON at the boot time, which wasn't
happening in case of a warm re-booting (i.e. by PMC resetting).

Changelog:

v2: - Extended the commit's message of the "Balance runtime PM use-count on
  resume failure" patch.

- Re-send for 5.9 inclusion.

Dmitry Osipenko (4):
  media: staging: tegra-vde: Balance runtime PM use-count on resume
failure
  media: staging: tegra-vde: Runtime PM is always available on Tegra
  media: staging: tegra-vde: Turn ON power domain on shutdown
  media: staging: tegra-vde: Power-cycle hardware on probe

 drivers/staging/media/tegra-vde/vde.c | 45 +--
 1 file changed, 29 insertions(+), 16 deletions(-)

-- 
2.26.0

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


[PATCH v2 4/4] media: staging: tegra-vde: Power-cycle hardware on probe

2020-06-24 Thread Dmitry Osipenko
VDE partition is left turned ON after bootloader on most devices, hence
let's ensure that it's turned OFF in order to lower power leakage while
hardware is idling by turning it ON and OFF during of the driver's probe.

Signed-off-by: Dmitry Osipenko 
---
 drivers/staging/media/tegra-vde/vde.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/drivers/staging/media/tegra-vde/vde.c 
b/drivers/staging/media/tegra-vde/vde.c
index b64e35b86fb4..3be96c36bf43 100644
--- a/drivers/staging/media/tegra-vde/vde.c
+++ b/drivers/staging/media/tegra-vde/vde.c
@@ -1068,6 +1068,14 @@ static int tegra_vde_probe(struct platform_device *pdev)
pm_runtime_use_autosuspend(dev);
pm_runtime_set_autosuspend_delay(dev, 300);
 
+   /*
+* VDE partition may be left ON after bootloader, hence let's
+* power-cycle it in order to put hardware into a predictable lower
+* power state.
+*/
+   pm_runtime_get_sync(dev);
+   pm_runtime_put(dev);
+
return 0;
 
 err_deinit_iommu:
-- 
2.26.0

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


[PATCH v2 3/4] media: staging: tegra-vde: Turn ON power domain on shutdown

2020-06-24 Thread Dmitry Osipenko
On some devices bootloader isn't ready to a clamped VDE power, and thus,
machine hangs on a warm reboot (CPU reset). The VDE power partition is
turned ON by default on a cold boot, hence VDE driver should keep power
partition enabled on system's reboot too. This fixes hang on a warm reboot
on a Tegra20 Acer A500 device, which is handy if Embedded Controller
driver is unavailable, i.e. cold reboot can't be performed.

Signed-off-by: Dmitry Osipenko 
---
 drivers/staging/media/tegra-vde/vde.c | 19 +++
 1 file changed, 19 insertions(+)

diff --git a/drivers/staging/media/tegra-vde/vde.c 
b/drivers/staging/media/tegra-vde/vde.c
index 85cbbc8f70d3..b64e35b86fb4 100644
--- a/drivers/staging/media/tegra-vde/vde.c
+++ b/drivers/staging/media/tegra-vde/vde.c
@@ -1085,9 +1085,17 @@ static int tegra_vde_remove(struct platform_device *pdev)
struct tegra_vde *vde = platform_get_drvdata(pdev);
struct device *dev = &pdev->dev;
 
+   pm_runtime_get_sync(dev);
pm_runtime_dont_use_autosuspend(dev);
pm_runtime_disable(dev);
 
+   /*
+* Balance RPM state, the VDE power domain is left ON and hardware
+* is clock-gated. It's safe to reboot machine now.
+*/
+   pm_runtime_put_noidle(dev);
+   clk_disable_unprepare(vde->clk);
+
misc_deregister(&vde->miscdev);
 
tegra_vde_dmabuf_cache_unmap_all(vde);
@@ -1099,6 +1107,16 @@ static int tegra_vde_remove(struct platform_device *pdev)
return 0;
 }
 
+static void tegra_vde_shutdown(struct platform_device *pdev)
+{
+   /*
+* On some devices bootloader isn't ready to a power-gated VDE on
+* a warm-reboot, machine will hang in that case.
+*/
+   if (pm_runtime_status_suspended(&pdev->dev))
+   tegra_vde_runtime_resume(&pdev->dev);
+}
+
 static __maybe_unused int tegra_vde_pm_suspend(struct device *dev)
 {
struct tegra_vde *vde = dev_get_drvdata(dev);
@@ -1144,6 +1162,7 @@ MODULE_DEVICE_TABLE(of, tegra_vde_of_match);
 static struct platform_driver tegra_vde_driver = {
.probe  = tegra_vde_probe,
.remove = tegra_vde_remove,
+   .shutdown   = tegra_vde_shutdown,
.driver = {
.name   = "tegra-vde",
.of_match_table = tegra_vde_of_match,
-- 
2.26.0

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


Re: [PATCH v2 0/4] Tegra Video Decoder driver power management corrections

2020-06-24 Thread Hans Verkuil
On 24/06/2020 17:08, Dmitry Osipenko wrote:
> Hello,
> 
> This small series addresses a Runtime PM issue that was discovered during
> of Tegra VI driver reviewing by balancing RPM usage count on RPM resume
> failure. Secondly it fixes reboot on some Tegra devices due to bootloader
> expecting VDE power partition to be ON at the boot time, which wasn't
> happening in case of a warm re-booting (i.e. by PMC resetting).

Can you rebase this on top of the media_tree master branch? I think a variant
of patch 1 has already been applied. I found a mail today where you mentioned
that you preferred your version (it looks like I missed that) so you'll need to
rework patch 1.

Sorry about this,

Hans

> 
> Changelog:
> 
> v2: - Extended the commit's message of the "Balance runtime PM use-count on
>   resume failure" patch.
> 
> - Re-send for 5.9 inclusion.
> 
> Dmitry Osipenko (4):
>   media: staging: tegra-vde: Balance runtime PM use-count on resume
> failure
>   media: staging: tegra-vde: Runtime PM is always available on Tegra
>   media: staging: tegra-vde: Turn ON power domain on shutdown
>   media: staging: tegra-vde: Power-cycle hardware on probe
> 
>  drivers/staging/media/tegra-vde/vde.c | 45 +--
>  1 file changed, 29 insertions(+), 16 deletions(-)
> 

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


Re: [PATCH v2 0/4] Tegra Video Decoder driver power management corrections

2020-06-24 Thread Dmitry Osipenko
24.06.2020 18:16, Hans Verkuil пишет:
> On 24/06/2020 17:08, Dmitry Osipenko wrote:
>> Hello,
>>
>> This small series addresses a Runtime PM issue that was discovered during
>> of Tegra VI driver reviewing by balancing RPM usage count on RPM resume
>> failure. Secondly it fixes reboot on some Tegra devices due to bootloader
>> expecting VDE power partition to be ON at the boot time, which wasn't
>> happening in case of a warm re-booting (i.e. by PMC resetting).
> 
> Can you rebase this on top of the media_tree master branch? I think a variant
> of patch 1 has already been applied. I found a mail today where you mentioned
> that you preferred your version (it looks like I missed that) so you'll need 
> to
> rework patch 1.

Hello Hans,

I'll take a look at what patches has been applied, my bad for sending
the v2 too late. Thank you for the heads up!
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH] Staging: rtl8188eu: Fix alignment coding style issue

2020-06-24 Thread Peilin Ye
Fix "Alignment should match open parenthesis" issues reported by
checkpatch.pl for all files under drivers/staging/rtl8188eu/core.

Line rtw_mlme_ext.c:373 is left overlength for readability.

Signed-off-by: Peilin Ye 
---
 drivers/staging/rtl8188eu/core/rtw_cmd.c   |  4 +--
 drivers/staging/rtl8188eu/core/rtw_debug.c | 20 +--
 drivers/staging/rtl8188eu/core/rtw_ieee80211.c | 17 +-
 drivers/staging/rtl8188eu/core/rtw_ioctl_set.c |  2 +-
 drivers/staging/rtl8188eu/core/rtw_mlme.c  |  4 +--
 drivers/staging/rtl8188eu/core/rtw_mlme_ext.c  | 46 --
 drivers/staging/rtl8188eu/core/rtw_pwrctrl.c   |  2 +-
 drivers/staging/rtl8188eu/core/rtw_recv.c  |  8 ++---
 8 files changed, 50 insertions(+), 53 deletions(-)

diff --git a/drivers/staging/rtl8188eu/core/rtw_cmd.c 
b/drivers/staging/rtl8188eu/core/rtw_cmd.c
index f69e9453ad45..a97d50081071 100644
--- a/drivers/staging/rtl8188eu/core/rtw_cmd.c
+++ b/drivers/staging/rtl8188eu/core/rtw_cmd.c
@@ -229,7 +229,7 @@ int rtw_cmd_thread(void *context)
  * LOCKED pmlmepriv->lock
  */
 u8 rtw_sitesurvey_cmd(struct adapter  *padapter, struct ndis_802_11_ssid 
*ssid, int ssid_num,
-   struct rtw_ieee80211_channel *ch, int ch_num)
+ struct rtw_ieee80211_channel *ch, int ch_num)
 {
u8 res = _FAIL;
struct cmd_obj  *ph2c;
@@ -449,7 +449,7 @@ u8 rtw_joinbss_cmd(struct adapter  *padapter, struct 
wlan_network *pnetwork)
(padapter->securitypriv.dot11PrivacyAlgrthm != _TKIP_)) {
/* rtw_restructure_ht_ie */
rtw_restructure_ht_ie(padapter, 
&pnetwork->network.ies[0], &psecnetwork->ies[0],
-   
pnetwork->network.ie_length, &psecnetwork->ie_length);
+ pnetwork->network.ie_length, 
&psecnetwork->ie_length);
}
}
 
diff --git a/drivers/staging/rtl8188eu/core/rtw_debug.c 
b/drivers/staging/rtl8188eu/core/rtw_debug.c
index d0e41f2ef1ce..fcc8bd1011e1 100644
--- a/drivers/staging/rtl8188eu/core/rtw_debug.c
+++ b/drivers/staging/rtl8188eu/core/rtw_debug.c
@@ -10,8 +10,8 @@
 #include 
 
 int proc_get_drv_version(char *page, char **start,
- off_t offset, int count,
- int *eof, void *data)
+off_t offset, int count,
+int *eof, void *data)
 {
int len = 0;
 
@@ -22,15 +22,15 @@ int proc_get_drv_version(char *page, char **start,
 }
 
 int proc_get_write_reg(char *page, char **start,
- off_t offset, int count,
- int *eof, void *data)
+  off_t offset, int count,
+  int *eof, void *data)
 {
*eof = 1;
return 0;
 }
 
 int proc_set_write_reg(struct file *file, const char __user *buffer,
-   unsigned long count, void *data)
+  unsigned long count, void *data)
 {
struct net_device *dev = data;
struct adapter *padapter = (struct adapter *)rtw_netdev_priv(dev);
@@ -71,8 +71,8 @@ static u32 proc_get_read_addr = 0x;
 static u32 proc_get_read_len = 0x4;
 
 int proc_get_read_reg(char *page, char **start,
- off_t offset, int count,
- int *eof, void *data)
+ off_t offset, int count,
+ int *eof, void *data)
 {
struct net_device *dev = data;
struct adapter *padapter = (struct adapter *)rtw_netdev_priv(dev);
@@ -104,7 +104,7 @@ int proc_get_read_reg(char *page, char **start,
 }
 
 int proc_set_read_reg(struct file *file, const char __user *buffer,
-   unsigned long count, void *data)
+ unsigned long count, void *data)
 {
char tmp[16];
u32 addr, len;
@@ -131,8 +131,8 @@ int proc_set_read_reg(struct file *file, const char __user 
*buffer,
 }
 
 int proc_get_adapter_state(char *page, char **start,
- off_t offset, int count,
- int *eof, void *data)
+  off_t offset, int count,
+  int *eof, void *data)
 {
struct net_device *dev = data;
struct adapter *padapter = (struct adapter *)rtw_netdev_priv(dev);
diff --git a/drivers/staging/rtl8188eu/core/rtw_ieee80211.c 
b/drivers/staging/rtl8188eu/core/rtw_ieee80211.c
index caf600eba03b..5630d5d763db 100644
--- a/drivers/staging/rtl8188eu/core/rtw_ieee80211.c
+++ b/drivers/staging/rtl8188eu/core/rtw_ieee80211.c
@@ -675,8 +675,7 @@ u8 *rtw_get_wps_attr_content(u8 *wps_ie, uint wps_ielen, 
u16 target_attr_id, u8
 }
 
 static int rtw_ieee802_11_parse_vendor_specific(u8 *pos, uint elen,
-   struct rtw_ieee802_11_elems *elems,
-   int show_errors)
+ 

[PATCH v2 2/2] staging: vc04_services: vchiq_arm: Remove unnecessary parens

2020-06-24 Thread Garrit Franke
Signed-off-by: Garrit Franke 

---
 drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c 
b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
index 5a6d2bd59ec0..e0027148963e 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
@@ -533,7 +533,7 @@ request_poll(struct vchiq_state *state, struct 
vchiq_service *service,
service->localport>>5]);
} while (atomic_cmpxchg(
&state->poll_services[service->localport>>5],
-   value, value | BIT((service->localport & 0x1f)))
+   value, value | BIT(service->localport & 0x1f))
!= value);
}
 
-- 
2.25.1

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


[PATCH v2 0/2] staging: vc04_services: vchiq_arm: replace bitshift with BIT macro

2020-06-24 Thread Garrit Franke
Hi Dan,

thanks for your review. Yes, this is supposed to be a cleanup. I didn't
make that clear. I removed the unnecessary parentheses, as suggested.

Regards,
Garrit Franke

Garrit Franke (2):
  staging: vc04_services: vchiq_arm: replace bitshift with BIT macro
  staging: vc04_services: vchiq_arm: Remove unnecessary parens

 .../interface/vchiq_arm/vchiq_core.c  | 22 +--
 1 file changed, 11 insertions(+), 11 deletions(-)

-- 
2.25.1

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


[PATCH v2 1/2] staging: vc04_services: vchiq_arm: replace bitshift with BIT macro

2020-06-24 Thread Garrit Franke
This should prevent possible overflowing bits by using the BIT macro in
vchiq_core

Signed-off-by: Garrit Franke 
---
 .../interface/vchiq_arm/vchiq_core.c  | 22 +--
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c 
b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
index ae9183db44ee..5a6d2bd59ec0 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
@@ -39,9 +39,9 @@ struct vchiq_openack_payload {
 };
 
 enum {
-   QMFLAGS_IS_BLOCKING = (1 << 0),
-   QMFLAGS_NO_MUTEX_LOCK   = (1 << 1),
-   QMFLAGS_NO_MUTEX_UNLOCK = (1 << 2)
+   QMFLAGS_IS_BLOCKING = BIT(0),
+   QMFLAGS_NO_MUTEX_LOCK   = BIT(1),
+   QMFLAGS_NO_MUTEX_UNLOCK = BIT(2)
 };
 
 /* we require this for consistency between endpoints */
@@ -526,14 +526,14 @@ request_poll(struct vchiq_state *state, struct 
vchiq_service *service,
do {
value = atomic_read(&service->poll_flags);
} while (atomic_cmpxchg(&service->poll_flags, value,
-   value | (1 << poll_type)) != value);
+   value | BIT(poll_type)) != value);
 
do {
value = atomic_read(&state->poll_services[
service->localport>>5]);
} while (atomic_cmpxchg(
&state->poll_services[service->localport>>5],
-   value, value | (1 << (service->localport & 0x1f)))
+   value, value | BIT((service->localport & 0x1f)))
!= value);
}
 
@@ -1287,19 +1287,19 @@ poll_services(struct vchiq_state *state)
 
flags = atomic_xchg(&state->poll_services[group], 0);
for (i = 0; flags; i++) {
-   if (flags & (1 << i)) {
+   if (flags & BIT(i)) {
struct vchiq_service *service =
find_service_by_port(state,
(group<<5) + i);
u32 service_flags;
 
-   flags &= ~(1 << i);
+   flags &= ~BIT(i);
if (!service)
continue;
service_flags =
atomic_xchg(&service->poll_flags, 0);
if (service_flags &
-   (1 << VCHIQ_POLL_REMOVE)) {
+   BIT(VCHIQ_POLL_REMOVE)) {
vchiq_log_info(vchiq_core_log_level,
"%d: ps - remove %d<->%d",
state->id, service->localport,
@@ -1317,7 +1317,7 @@ poll_services(struct vchiq_state *state)
request_poll(state, service,
VCHIQ_POLL_REMOVE);
} else if (service_flags &
-   (1 << VCHIQ_POLL_TERMINATE)) {
+   BIT(VCHIQ_POLL_TERMINATE)) {
vchiq_log_info(vchiq_core_log_level,
"%d: ps - terminate %d<->%d",
state->id, service->localport,
@@ -1328,11 +1328,11 @@ poll_services(struct vchiq_state *state)
request_poll(state, service,
VCHIQ_POLL_TERMINATE);
}
-   if (service_flags & (1 << VCHIQ_POLL_TXNOTIFY))
+   if (service_flags & BIT(VCHIQ_POLL_TXNOTIFY))
notify_bulks(service,
&service->bulk_tx,
1/*retry_poll*/);
-   if (service_flags & (1 << VCHIQ_POLL_RXNOTIFY))
+   if (service_flags & BIT(VCHIQ_POLL_RXNOTIFY))
notify_bulks(service,
&service->bulk_rx,
1/*retry_poll*/);
-- 
2.25.1

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