Re: [PATCH v7 00/17] wilc1000: move out of staging
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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