[PATCH] staging: rtl8188eu: break line longer than 80 cols
Break too long line to follow kernel coding style. Signed-off-by: Kacper Kołodziej --- drivers/staging/rtl8188eu/core/rtw_ap.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/staging/rtl8188eu/core/rtw_ap.c b/drivers/staging/rtl8188eu/core/rtw_ap.c index 454a975a14f2..220b4bbe1f84 100644 --- a/drivers/staging/rtl8188eu/core/rtw_ap.c +++ b/drivers/staging/rtl8188eu/core/rtw_ap.c @@ -362,7 +362,8 @@ voidexpire_timeout_chk(struct adapter *padapter) stainfo_offset = rtw_stainfo_offset(pstapriv, psta); if (stainfo_offset_valid(stainfo_offset)) - chk_alive_list[chk_alive_num++] = stainfo_offset; + chk_alive_list[chk_alive_num++] = + stainfo_offset; continue; } -- 2.16.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/5] staging: rtl8723bs: remove unused code
Remove commented rtw_is_cckrates_included() and rtw_is_cckratesonly_included() from os_dep/ioctl_linux.c. Both are defined in core/rtw_ieee80211.c. Signed-off-by: Michael Straube --- .../staging/rtl8723bs/os_dep/ioctl_linux.c| 32 --- 1 file changed, 32 deletions(-) diff --git a/drivers/staging/rtl8723bs/os_dep/ioctl_linux.c b/drivers/staging/rtl8723bs/os_dep/ioctl_linux.c index ceb2b10fa0b2..63b79a7eaccf 100644 --- a/drivers/staging/rtl8723bs/os_dep/ioctl_linux.c +++ b/drivers/staging/rtl8723bs/os_dep/ioctl_linux.c @@ -79,38 +79,6 @@ void rtw_indicate_wx_disassoc_event(struct adapter *padapter) eth_zero_addr(wrqu.ap_addr.sa_data); } -/* -uint rtw_is_cckrates_included(u8 *rate) -{ - u32 i = 0; - - while (rate[i]!= 0) - { - if rate[i]) & 0x7f) == 2) || (((rate[i]) & 0x7f) == 4) || - (((rate[i]) & 0x7f) == 11) || (((rate[i]) & 0x7f) == 22)) - return true; - i++; - } - - return false; -} - -uint rtw_is_cckratesonly_included(u8 *rate) -{ - u32 i = 0; - - while (rate[i]!= 0) - { - if rate[i]) & 0x7f) != 2) && (((rate[i]) & 0x7f) != 4) && - (((rate[i]) & 0x7f) != 11) && (((rate[i]) & 0x7f) != 22)) - return false; - i++; - } - - return true; -} -*/ - static char *translate_scan(struct adapter *padapter, struct iw_request_info* info, struct wlan_network *pnetwork, char *start, char *stop) -- 2.18.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/5] staging: rtl8723bs: refactor rtw_is_cckrates_included()
Refactor rtw_is_cckrates_included() to improve readability and slightly reduce object file size. Suggested-by: Joe Perches Signed-off-by: Michael Straube --- drivers/staging/rtl8723bs/core/rtw_ieee80211.c | 9 - 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/drivers/staging/rtl8723bs/core/rtw_ieee80211.c b/drivers/staging/rtl8723bs/core/rtw_ieee80211.c index 3f1c7bb0eb9f..3adb58759d5f 100644 --- a/drivers/staging/rtl8723bs/core/rtw_ieee80211.c +++ b/drivers/staging/rtl8723bs/core/rtw_ieee80211.c @@ -68,13 +68,12 @@ int rtw_get_bit_value_from_ieee_value(u8 val) uint rtw_is_cckrates_included(u8 *rate) { - u32 i = 0; + while (*rate) { + u8 r = *rate & 0x7f; - while (rate[i] != 0) { - if rate[i]) & 0x7f) == 2) || (((rate[i]) & 0x7f) == 4) || -(((rate[i]) & 0x7f) == 11) || (((rate[i]) & 0x7f) == 22)) + if (r == 2 || r == 4 || r == 11 || r == 22) return true; - i++; + rate++; } return false; -- 2.18.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 5/5] staging: rtl8723bs: fix comparsions to true
Use if(x) instead of if(x == true). Signed-off-by: Michael Straube --- .../staging/rtl8723bs/core/rtw_ieee80211.c| 6 ++-- .../staging/rtl8723bs/os_dep/ioctl_linux.c| 28 +-- 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/drivers/staging/rtl8723bs/core/rtw_ieee80211.c b/drivers/staging/rtl8723bs/core/rtw_ieee80211.c index 62718045f3cc..33f2649ba2ec 100644 --- a/drivers/staging/rtl8723bs/core/rtw_ieee80211.c +++ b/drivers/staging/rtl8723bs/core/rtw_ieee80211.c @@ -95,14 +95,14 @@ bool rtw_is_cckratesonly_included(u8 *rate) int rtw_check_network_type(unsigned char *rate, int ratelen, int channel) { if (channel > 14) { - if ((rtw_is_cckrates_included(rate)) == true) + if (rtw_is_cckrates_included(rate)) return WIRELESS_INVALID; else return WIRELESS_11A; } else{ /* could be pure B, pure G, or B/G */ - if ((rtw_is_cckratesonly_included(rate)) == true) + if (rtw_is_cckratesonly_included(rate)) return WIRELESS_11B; - else if ((rtw_is_cckrates_included(rate)) == true) + else if (rtw_is_cckrates_included(rate)) return WIRELESS_11BG; else return WIRELESS_11G; diff --git a/drivers/staging/rtl8723bs/os_dep/ioctl_linux.c b/drivers/staging/rtl8723bs/os_dep/ioctl_linux.c index 63b79a7eaccf..c38298d960ff 100644 --- a/drivers/staging/rtl8723bs/os_dep/ioctl_linux.c +++ b/drivers/staging/rtl8723bs/os_dep/ioctl_linux.c @@ -126,26 +126,26 @@ static char *translate_scan(struct adapter *padapter, /* Add the protocol name */ iwe.cmd = SIOCGIWNAME; - if ((rtw_is_cckratesonly_included((u8 *)&pnetwork->network.SupportedRates)) == true) { - if (ht_cap == true) + if (rtw_is_cckratesonly_included((u8 *)&pnetwork->network.SupportedRates)) { + if (ht_cap) snprintf(iwe.u.name, IFNAMSIZ, "IEEE 802.11bn"); else snprintf(iwe.u.name, IFNAMSIZ, "IEEE 802.11b"); - } else if ((rtw_is_cckrates_included((u8 *)&pnetwork->network.SupportedRates)) == true) { - if (ht_cap == true) + } else if (rtw_is_cckrates_included((u8 *)&pnetwork->network.SupportedRates)) { + if (ht_cap) snprintf(iwe.u.name, IFNAMSIZ, "IEEE 802.11bgn"); else snprintf(iwe.u.name, IFNAMSIZ, "IEEE 802.11bg"); } else { if (pnetwork->network.Configuration.DSConfig > 14) { - if (vht_cap == true) + if (vht_cap) snprintf(iwe.u.name, IFNAMSIZ, "IEEE 802.11AC"); - else if (ht_cap == true) + else if (ht_cap) snprintf(iwe.u.name, IFNAMSIZ, "IEEE 802.11an"); else snprintf(iwe.u.name, IFNAMSIZ, "IEEE 802.11a"); } else { - if (ht_cap == true) + if (ht_cap) snprintf(iwe.u.name, IFNAMSIZ, "IEEE 802.11gn"); else snprintf(iwe.u.name, IFNAMSIZ, "IEEE 802.11g"); @@ -785,26 +785,26 @@ static int rtw_wx_get_name(struct net_device *dev, prates = &pcur_bss->SupportedRates; - if (rtw_is_cckratesonly_included((u8 *)prates) == true) { - if (ht_cap == true) + if (rtw_is_cckratesonly_included((u8 *)prates)) { + if (ht_cap) snprintf(wrqu->name, IFNAMSIZ, "IEEE 802.11bn"); else snprintf(wrqu->name, IFNAMSIZ, "IEEE 802.11b"); - } else if ((rtw_is_cckrates_included((u8 *)prates)) == true) { - if (ht_cap == true) + } else if (rtw_is_cckrates_included((u8 *)prates)) { + if (ht_cap) snprintf(wrqu->name, IFNAMSIZ, "IEEE 802.11bgn"); else snprintf(wrqu->name, IFNAMSIZ, "IEEE 802.11bg"); } else { if (pcur_bss->Configuration.DSConfig > 14) { - if (vht_cap == true) + if (vht_cap) snprintf(wrqu->name, IFNAMSIZ, "IEEE 802.11AC"); - else if (ht_cap == true) + else if (ht_cap) snprintf(wrqu->name, IFNAMSIZ, "IEEE 802.11an"); else snprintf(wrqu->name, IFNAMSIZ, "IEEE 802.11a");
[PATCH 4/5] staging: rtl8723bs: change return type to bool
Both rtw_is_cckrates_included() and rtw_is_cckratesonly_included() return true or false. Change the return type from uint to bool. Signed-off-by: Michael Straube --- drivers/staging/rtl8723bs/core/rtw_ieee80211.c | 4 ++-- drivers/staging/rtl8723bs/include/ieee80211.h | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/staging/rtl8723bs/core/rtw_ieee80211.c b/drivers/staging/rtl8723bs/core/rtw_ieee80211.c index e4a20a4a5e59..62718045f3cc 100644 --- a/drivers/staging/rtl8723bs/core/rtw_ieee80211.c +++ b/drivers/staging/rtl8723bs/core/rtw_ieee80211.c @@ -66,7 +66,7 @@ int rtw_get_bit_value_from_ieee_value(u8 val) return 0; } -uint rtw_is_cckrates_included(u8 *rate) +bool rtw_is_cckrates_included(u8 *rate) { while (*rate) { u8 r = *rate & 0x7f; @@ -79,7 +79,7 @@ uint rtw_is_cckrates_included(u8 *rate) return false; } -uint rtw_is_cckratesonly_included(u8 *rate) +bool rtw_is_cckratesonly_included(u8 *rate) { while (*rate) { u8 r = *rate & 0x7f; diff --git a/drivers/staging/rtl8723bs/include/ieee80211.h b/drivers/staging/rtl8723bs/include/ieee80211.h index 974e922f54fa..bcc8dfa8e672 100644 --- a/drivers/staging/rtl8723bs/include/ieee80211.h +++ b/drivers/staging/rtl8723bs/include/ieee80211.h @@ -1169,9 +1169,9 @@ int rtw_generate_ie(struct registry_priv *pregistrypriv); int rtw_get_bit_value_from_ieee_value(u8 val); -uint rtw_is_cckrates_included(u8 *rate); +bool rtw_is_cckrates_included(u8 *rate); -uint rtw_is_cckratesonly_included(u8 *rate); +bool rtw_is_cckratesonly_included(u8 *rate); int rtw_check_network_type(unsigned char *rate, int ratelen, int channel); -- 2.18.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 3/5] staging: rtl8723bs: refactor rtw_is_cckratesonly_included
Refactor rtw_is_cckratesonly_included() to improve readability and slightly reduce object file size. Suggested-by: Joe Perches Signed-off-by: Michael Straube --- drivers/staging/rtl8723bs/core/rtw_ieee80211.c | 9 - 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/drivers/staging/rtl8723bs/core/rtw_ieee80211.c b/drivers/staging/rtl8723bs/core/rtw_ieee80211.c index 3adb58759d5f..e4a20a4a5e59 100644 --- a/drivers/staging/rtl8723bs/core/rtw_ieee80211.c +++ b/drivers/staging/rtl8723bs/core/rtw_ieee80211.c @@ -81,13 +81,12 @@ uint rtw_is_cckrates_included(u8 *rate) uint rtw_is_cckratesonly_included(u8 *rate) { - u32 i = 0; + while (*rate) { + u8 r = *rate & 0x7f; - while (rate[i] != 0) { - if rate[i]) & 0x7f) != 2) && (((rate[i]) & 0x7f) != 4) && -(((rate[i]) & 0x7f) != 11) && (((rate[i]) & 0x7f) != 22)) + if (r != 2 && r != 4 && r != 11 && r != 22) return false; - i++; + rate++; } return true; -- 2.18.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: ks7010: pass 'int' instead of 'bool' to 'hostif_mib_set_request_bool'
On Thu, Jul 12, 2018 at 06:57:42AM +0200, Sergio Paracuellos wrote: > 'hostif_mib_set_request_bool' function receives a bool as value and > send the received value with MIB_VALUE_TYPE_BOOL type. There is > one case where the value passed is not a boolean one but > 'MCAST_FILTER_PROMISC' which is '2'. Pass 'int' instead to avoid > the problem. > > Fixes: 8ce76bff0e6a ("staging: ks7010: add new helpers to achieve > mib set request and simplify code") > > Reported-by: Dan Carpenter > Signed-off-by: Sergio Paracuellos > --- > drivers/staging/ks7010/ks_hostif.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/staging/ks7010/ks_hostif.c > b/drivers/staging/ks7010/ks_hostif.c > index 0ecffab..6582566 100644 > --- a/drivers/staging/ks7010/ks_hostif.c > +++ b/drivers/staging/ks7010/ks_hostif.c > @@ -1228,7 +1228,7 @@ static inline void hostif_mib_set_request_int(struct > ks_wlan_private *priv, > > static inline void hostif_mib_set_request_bool(struct ks_wlan_private *priv, > enum mib_attribute attr, > -bool val) > +int val) Huh... This doesn't feel like the right thing. I thought we should change the callers to use hostif_mib_set_request_int() instead. This reverts to the original behavior, yes, but I think the original was wrong. This is one of those changes which requires someone to test it probably. Or at least understand the code more than you and I do... regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 1/2] staging: goldfish: fix whitespace in goldfish_audio
On Wed, Jul 11, 2018 at 04:36:28PM -0700, r...@google.com wrote: > From: Roman Kiryanov > > Linux kernel coding style: spaces are never used for > indentation. This patch is good but that's not strictly true... I don't think the code here causes a checkpatch.pl warning, and checkpatch is pretty on point with regards to tabs vs spaces. The rules are: 1) Use tabs to indent code 2) Tabs always come before spaces 3) Use spaces to make things align So say you have an if statement: if (some_condition() || bar) { blah blah blah The bar line is [tab][space][space][space][space]bar) { It's the same for function parameters, you have to use spaces to make them align. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: rtl8188eu: use strlcpy instead of strncpy
Use strlcpy instead of strncpy to avoid gcc 8 warning: warning: '__builtin_strncpy' specified bound 16 equals destination size [-Wstringop-truncation] The maximum length of the source string including terminating null is 5. Hence it always fits in the destination buffer of length 16. Signed-off-by: Michael Straube --- drivers/staging/rtl8188eu/os_dep/ioctl_linux.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c b/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c index 80ccd19e776d..0f82a8c330f8 100644 --- a/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c +++ b/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c @@ -1912,7 +1912,7 @@ static int rtw_wx_set_enc_ext(struct net_device *dev, goto exit; } - strncpy((char *)param->u.crypt.alg, alg_name, IEEE_CRYPT_ALG_NAME_LEN); + strlcpy((char *)param->u.crypt.alg, alg_name, IEEE_CRYPT_ALG_NAME_LEN); if (pext->ext_flags & IW_ENCODE_EXT_SET_TX_KEY) param->u.crypt.set_tx = 1; -- 2.18.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/2] staging: goldfish: fix whitespace in goldfish_audio
> > Linux kernel coding style: spaces are never used for > > indentation. > > ... > > struct goldfish_audio { > > char __iomem *reg_base; > > int irq; > > + > > That's not a space/tab issue? Thank you for looking. I separated the new line change into a separate patch. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 1/2] staging: mt7621-pci: use generic kernel pci subsystem read and write
map_bus callback is called before every .read/.write operation. Implement it and change custom read write operations for the pci subsystem generics. Make the probe function to assign data for controller data and get pci register base from device tree. Signed-off-by: Sergio Paracuellos --- drivers/staging/mt7621-pci/pci-mt7621.c | 99 +++-- 1 file changed, 95 insertions(+), 4 deletions(-) diff --git a/drivers/staging/mt7621-pci/pci-mt7621.c b/drivers/staging/mt7621-pci/pci-mt7621.c index 4f56840..6187d41 100644 --- a/drivers/staging/mt7621-pci/pci-mt7621.c +++ b/drivers/staging/mt7621-pci/pci-mt7621.c @@ -52,6 +52,9 @@ #include #include #include +#include +#include +#include #include #include @@ -177,6 +180,32 @@ static int pcie_link_status = 0; #define PCI_ACCESS_WRITE_2 4 #define PCI_ACCESS_WRITE_4 5 +/** + * struct mt7621_pcie_port - PCIe port information + * @base: IO mapped register base + * @list: port list + * @pcie: pointer to PCIe host info + * @reset: pointer to port reset control + */ +struct mt7621_pcie_port { + void __iomem *base; + struct list_head list; + struct mt7621_pcie *pcie; + struct reset_control *reset; +}; + +/** + * struct mt7621_pcie - PCIe host information + * @base: IO Mapped Register Base + * @dev: Pointer to PCIe device + * @ports: pointer to PCIe port information + */ +struct mt7621_pcie { + void __iomem *base; + struct device *dev; + struct list_head ports; +}; + static inline u32 mt7621_pci_get_cfgaddr(unsigned int bus, unsigned int slot, unsigned int func, unsigned int where) { @@ -296,15 +325,27 @@ pci_config_write(struct pci_bus *bus, unsigned int devfn, int where, int size, u } } +static void __iomem *mt7621_pcie_map_bus(struct pci_bus *bus, +unsigned int devfn, int where) +{ + struct mt7621_pcie *pcie = bus->sysdata; + u32 address = mt7621_pci_get_cfgaddr(bus->number, PCI_SLOT(devfn), +PCI_FUNC(devfn), where); + + writel(address, pcie->base + RALINK_PCI_CONFIG_ADDR); + + return pcie->base + RALINK_PCI_CONFIG_DATA_VIRTUAL_REG + (where & 3); +} + struct pci_ops mt7621_pci_ops = { - .read = pci_config_read, - .write = pci_config_write, + .map_bus= mt7621_pcie_map_bus, + .read = pci_generic_config_read, + .write = pci_generic_config_write, }; static struct resource mt7621_res_pci_mem1; static struct resource mt7621_res_pci_io1; static struct pci_controller mt7621_controller = { - .pci_ops= &mt7621_pci_ops, .mem_resource = &mt7621_res_pci_mem1, .io_resource= &mt7621_res_pci_io1, }; @@ -479,10 +520,61 @@ void setup_cm_memory_region(struct resource *mem_resource) } } +static int mt7621_pcie_parse_dt(struct mt7621_pcie *pcie) +{ + struct device *dev = pcie->dev; + struct device_node *node = dev->of_node; + struct resource regs; + const char *type; + int err; + + type = of_get_property(node, "device_type", NULL); + if (!type || strcmp(type, "pci") != 0) { + dev_err(dev, "invalid \"device_type\" %s\n", type); + return -EINVAL; + } + + err = of_address_to_resource(node, 0, ®s); + if (err) { + dev_err(dev, "missing \"reg\" property\n"); + return err; + } + + pcie->base = devm_pci_remap_cfg_resource(dev, ®s); + if (IS_ERR(pcie->base)) + return PTR_ERR(pcie->base); + + return 0; +} + static int mt7621_pci_probe(struct platform_device *pdev) { + struct device *dev = &pdev->dev; + struct mt7621_pcie *pcie; + struct pci_host_bridge *bridge; + int err; unsigned long val = 0; + bridge = devm_pci_alloc_host_bridge(dev, sizeof(*pcie)); + if (!bridge) + return -ENODEV; + + pcie = pci_host_bridge_priv(bridge); + pcie->dev = dev; + INIT_LIST_HEAD(&pcie->ports); + + err = mt7621_pcie_parse_dt(pcie); + if (err) { + dev_err(dev, "Parsing DT failed\n"); + return err; + } + + bridge->dev.parent = dev; + bridge->sysdata = pcie; + bridge->ops = &mt7621_pci_ops; + mt7621_controller.bus = bridge->bus; + mt7621_controller.bus->ops = bridge->ops; + iomem_resource.start = 0; iomem_resource.end = ~0; ioport_resource.start = 0; @@ -668,7 +760,6 @@ pcie(2/1/0) link status pcie2_num pcie1_num pcie0_num setup_cm_memory_region(mt7621_controller.mem_resource); register_pci_controller(&mt7621_controller); return 0; - } int pcibios_plat_dev_init(struct pci_dev *dev) -- 2.7.4 ___ devel mailing list de...@linuxdriverpr
[PATCH v2 2/2] staging: mt7621-pci: remove dead code derived to not use custom reads and writes
Driver is using now pci subsystem generics reads and writes. Because of this there is a lot of dead code that can be removed. Signed-off-by: Sergio Paracuellos --- drivers/staging/mt7621-pci/pci-mt7621.c | 128 1 file changed, 128 deletions(-) diff --git a/drivers/staging/mt7621-pci/pci-mt7621.c b/drivers/staging/mt7621-pci/pci-mt7621.c index 6187d41..17fdef9 100644 --- a/drivers/staging/mt7621-pci/pci-mt7621.c +++ b/drivers/staging/mt7621-pci/pci-mt7621.c @@ -120,15 +120,6 @@ *(volatile u32 *)(RALINK_PCI_BASE+(ofs)) = cpu_to_le32(data) #define MV_READ(ofs, data) \ *(data) = le32_to_cpu(*(volatile u32 *)(RALINK_PCI_BASE+(ofs))) -#define MV_WRITE_16(ofs, data) \ - *(volatile u16 *)(RALINK_PCI_BASE+(ofs)) = cpu_to_le16(data) -#define MV_READ_16(ofs, data) \ - *(data) = le16_to_cpu(*(volatile u16 *)(RALINK_PCI_BASE+(ofs))) - -#define MV_WRITE_8(ofs, data) \ - *(volatile u8 *)(RALINK_PCI_BASE+(ofs)) = data -#define MV_READ_8(ofs, data) \ - *(data) = *(volatile u8 *)(RALINK_PCI_BASE+(ofs)) #define RALINK_PCI_MM_MAP_BASE 0x6000 #define RALINK_PCI_IO_MAP_BASE 0x1e16 @@ -173,13 +164,6 @@ #define MEMORY_BASE 0x0 static int pcie_link_status = 0; -#define PCI_ACCESS_READ_1 0 -#define PCI_ACCESS_READ_2 1 -#define PCI_ACCESS_READ_4 2 -#define PCI_ACCESS_WRITE_1 3 -#define PCI_ACCESS_WRITE_2 4 -#define PCI_ACCESS_WRITE_4 5 - /** * struct mt7621_pcie_port - PCIe port information * @base: IO mapped register base @@ -213,118 +197,6 @@ static inline u32 mt7621_pci_get_cfgaddr(unsigned int bus, unsigned int slot, (func << 8) | (where & 0xfc) | 0x8000; } -static int config_access(unsigned char access_type, struct pci_bus *bus, - unsigned int devfn, unsigned int where, u32 *data) -{ - unsigned int slot = PCI_SLOT(devfn); - u8 func = PCI_FUNC(devfn); - u32 address_reg, data_reg; - unsigned int address; - - address_reg = RALINK_PCI_CONFIG_ADDR; - data_reg = RALINK_PCI_CONFIG_DATA_VIRTUAL_REG; - - address = mt7621_pci_get_cfgaddr(bus->number, slot, func, where); - - MV_WRITE(address_reg, address); - - switch (access_type) { - case PCI_ACCESS_WRITE_1: - MV_WRITE_8(data_reg+(where&0x3), *data); - break; - case PCI_ACCESS_WRITE_2: - MV_WRITE_16(data_reg+(where&0x3), *data); - break; - case PCI_ACCESS_WRITE_4: - MV_WRITE(data_reg, *data); - break; - case PCI_ACCESS_READ_1: - MV_READ_8(data_reg+(where&0x3), data); - break; - case PCI_ACCESS_READ_2: - MV_READ_16(data_reg+(where&0x3), data); - break; - case PCI_ACCESS_READ_4: - MV_READ(data_reg, data); - break; - default: - printk("no specify access type\n"); - break; - } - return 0; -} - -static int -read_config_byte(struct pci_bus *bus, unsigned int devfn, int where, u8 *val) -{ - return config_access(PCI_ACCESS_READ_1, bus, devfn, (unsigned int)where, (u32 *)val); -} - -static int -read_config_word(struct pci_bus *bus, unsigned int devfn, int where, u16 *val) -{ - return config_access(PCI_ACCESS_READ_2, bus, devfn, (unsigned int)where, (u32 *)val); -} - -static int -read_config_dword(struct pci_bus *bus, unsigned int devfn, int where, u32 *val) -{ - return config_access(PCI_ACCESS_READ_4, bus, devfn, (unsigned int)where, (u32 *)val); -} - -static int -write_config_byte(struct pci_bus *bus, unsigned int devfn, int where, u8 val) -{ - if (config_access(PCI_ACCESS_WRITE_1, bus, devfn, (unsigned int)where, (u32 *)&val)) - return -1; - - return PCIBIOS_SUCCESSFUL; -} - -static int -write_config_word(struct pci_bus *bus, unsigned int devfn, int where, u16 val) -{ - if (config_access(PCI_ACCESS_WRITE_2, bus, devfn, where, (u32 *)&val)) - return -1; - - return PCIBIOS_SUCCESSFUL; -} - -static int -write_config_dword(struct pci_bus *bus, unsigned int devfn, int where, u32 val) -{ - if (config_access(PCI_ACCESS_WRITE_4, bus, devfn, where, &val)) - return -1; - - return PCIBIOS_SUCCESSFUL; -} - -static int -pci_config_read(struct pci_bus *bus, unsigned int devfn, int where, int size, u32 *val) -{ - switch (size) { - case 1: - return read_config_byte(bus, devfn, where, (u8 *) val); - case 2: - return read_config_word(bus, devfn, where, (u16 *) val); - default: - return read_config_dword(bus, devfn, where, val); - } -} - -static int -pci_config_write(struct pci_bus *bus, unsigned int devfn, int where, int size, u32 val) -{ - switch (size) { - case 1: - return write_config_byte(bus, devfn, where, (u8) val); - case 2: -
[PATCH v2 0/2] staging: mt7621-pci: avoid custom pci config read and writes
This patch series include an attempt to avoid the use of custom read and writes in driver code and use PCI subsystem common ones. In order to do this 'map_bus' callback is implemented and also data structures for driver are included. The regs base address is being readed from device tree and the driver gets clean a lot of code. Changes in v2: - squash PATCH 1 and PATCH 2 of previous series in only PATCH 1 - Change name for host structure. - Create a new port structure (platform has 3 pcie controllers) - Replace the use of pci_generic_config_[read|write]32 in favour of pci_generic_config_[read|write] and change map_bus implemen- tation for hopefully the right one. I think this 'map_bus' function and pci read write generics should work, Neil. Please, test this series first and ignore the previous one. Thanks in advance for your time. Best regards, Sergio Paracuellos Sergio Paracuellos (2): staging: mt7621-pci: use generic kernel pci subsystem read and write staging: mt7621-pci: remove dead code derived to not use custom reads and writes drivers/staging/mt7621-pci/pci-mt7621.c | 215 +--- 1 file changed, 89 insertions(+), 126 deletions(-) -- 2.7.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: ks7010: pass 'int' instead of 'bool' to 'hostif_mib_set_request_bool'
On Thu, Jul 12, 2018 at 2:13 PM, Dan Carpenter wrote: > On Thu, Jul 12, 2018 at 06:57:42AM +0200, Sergio Paracuellos wrote: >> 'hostif_mib_set_request_bool' function receives a bool as value and >> send the received value with MIB_VALUE_TYPE_BOOL type. There is >> one case where the value passed is not a boolean one but >> 'MCAST_FILTER_PROMISC' which is '2'. Pass 'int' instead to avoid >> the problem. >> >> Fixes: 8ce76bff0e6a ("staging: ks7010: add new helpers to achieve >> mib set request and simplify code") >> >> Reported-by: Dan Carpenter >> Signed-off-by: Sergio Paracuellos >> --- >> drivers/staging/ks7010/ks_hostif.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/staging/ks7010/ks_hostif.c >> b/drivers/staging/ks7010/ks_hostif.c >> index 0ecffab..6582566 100644 >> --- a/drivers/staging/ks7010/ks_hostif.c >> +++ b/drivers/staging/ks7010/ks_hostif.c >> @@ -1228,7 +1228,7 @@ static inline void hostif_mib_set_request_int(struct >> ks_wlan_private *priv, >> >> static inline void hostif_mib_set_request_bool(struct ks_wlan_private *priv, >> enum mib_attribute attr, >> -bool val) >> +int val) > > Huh... This doesn't feel like the right thing. I thought we should > change the callers to use hostif_mib_set_request_int() instead. Yes, I though to call that instead at first moment but I end up in revert to the previous behaviour... > > This reverts to the original behavior, yes, but I think the original was > wrong. This is one of those changes which requires someone to test it > probably. Or at least understand the code more than you and I do... > I think is wrong also but I cannot be sure so... just being conservative this time :) > regards, > dan carpenter > Best regards, Sergio Paracuellos ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 1/2] staging: mt7621-pci: use generic kernel pci subsystem read and write
On Thu, Jul 12, 2018 at 08:15:00PM +0200, Sergio Paracuellos wrote: > map_bus callback is called before every .read/.write operation. > Implement it and change custom read write operations for the > pci subsystem generics. Make the probe function to assign data > for controller data and get pci register base from device tree. > > Signed-off-by: Sergio Paracuellos > --- > drivers/staging/mt7621-pci/pci-mt7621.c | 99 > +++-- > 1 file changed, 95 insertions(+), 4 deletions(-) > > diff --git a/drivers/staging/mt7621-pci/pci-mt7621.c > b/drivers/staging/mt7621-pci/pci-mt7621.c > index 4f56840..6187d41 100644 > --- a/drivers/staging/mt7621-pci/pci-mt7621.c > +++ b/drivers/staging/mt7621-pci/pci-mt7621.c > @@ -52,6 +52,9 @@ > #include > #include > #include > +#include > +#include > +#include > #include > > #include > @@ -177,6 +180,32 @@ static int pcie_link_status = 0; > #define PCI_ACCESS_WRITE_2 4 > #define PCI_ACCESS_WRITE_4 5 > > +/** > + * struct mt7621_pcie_port - PCIe port information > + * @base: IO mapped register base > + * @list: port list > + * @pcie: pointer to PCIe host info > + * @reset: pointer to port reset control > + */ > +struct mt7621_pcie_port { > + void __iomem *base; > + struct list_head list; > + struct mt7621_pcie *pcie; > + struct reset_control *reset; > +}; > + > +/** > + * struct mt7621_pcie - PCIe host information > + * @base: IO Mapped Register Base > + * @dev: Pointer to PCIe device > + * @ports: pointer to PCIe port information > + */ > +struct mt7621_pcie { > + void __iomem *base; > + struct device *dev; > + struct list_head ports; > +}; > + > static inline u32 mt7621_pci_get_cfgaddr(unsigned int bus, unsigned int slot, >unsigned int func, unsigned int where) > { > @@ -296,15 +325,27 @@ pci_config_write(struct pci_bus *bus, unsigned int > devfn, int where, int size, u > } > } > > +static void __iomem *mt7621_pcie_map_bus(struct pci_bus *bus, > + unsigned int devfn, int where) > +{ > + struct mt7621_pcie *pcie = bus->sysdata; > + u32 address = mt7621_pci_get_cfgaddr(bus->number, PCI_SLOT(devfn), > + PCI_FUNC(devfn), where); > + > + writel(address, pcie->base + RALINK_PCI_CONFIG_ADDR); > + > + return pcie->base + RALINK_PCI_CONFIG_DATA_VIRTUAL_REG + (where & 3); > +} > + > struct pci_ops mt7621_pci_ops = { > - .read = pci_config_read, > - .write = pci_config_write, > + .map_bus= mt7621_pcie_map_bus, > + .read = pci_generic_config_read, > + .write = pci_generic_config_write, > }; > > static struct resource mt7621_res_pci_mem1; > static struct resource mt7621_res_pci_io1; > static struct pci_controller mt7621_controller = { > - .pci_ops= &mt7621_pci_ops, > .mem_resource = &mt7621_res_pci_mem1, > .io_resource= &mt7621_res_pci_io1, > }; > @@ -479,10 +520,61 @@ void setup_cm_memory_region(struct resource > *mem_resource) > } > } > > +static int mt7621_pcie_parse_dt(struct mt7621_pcie *pcie) > +{ > + struct device *dev = pcie->dev; > + struct device_node *node = dev->of_node; > + struct resource regs; > + const char *type; > + int err; > + > + type = of_get_property(node, "device_type", NULL); > + if (!type || strcmp(type, "pci") != 0) { > + dev_err(dev, "invalid \"device_type\" %s\n", type); > + return -EINVAL; > + } > + > + err = of_address_to_resource(node, 0, ®s); > + if (err) { > + dev_err(dev, "missing \"reg\" property\n"); > + return err; > + } > + > + pcie->base = devm_pci_remap_cfg_resource(dev, ®s); > + if (IS_ERR(pcie->base)) > + return PTR_ERR(pcie->base); > + > + return 0; > +} > + > static int mt7621_pci_probe(struct platform_device *pdev) > { > + struct device *dev = &pdev->dev; > + struct mt7621_pcie *pcie; > + struct pci_host_bridge *bridge; > + int err; > unsigned long val = 0; > > + bridge = devm_pci_alloc_host_bridge(dev, sizeof(*pcie)); > + if (!bridge) > + return -ENODEV; > + > + pcie = pci_host_bridge_priv(bridge); > + pcie->dev = dev; > + INIT_LIST_HEAD(&pcie->ports); > + > + err = mt7621_pcie_parse_dt(pcie); > + if (err) { > + dev_err(dev, "Parsing DT failed\n"); We need to free bridge on this error path. > + return err; > + } regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: fsl-dpaa2/eth: Remove unnecessary cast
There's no need to explicitly cast DPAA2_ETH_MFL to u16, so remove it. Signed-off-by: Ioana Radulescu Suggested-by: Dan Carpenter --- drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c b/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c index 4ae2371..da993ed 100644 --- a/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c +++ b/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c @@ -2366,7 +2366,7 @@ static int netdev_init(struct net_device *net_dev) /* Set MTU upper limit; lower limit is 68B (default value) */ net_dev->max_mtu = DPAA2_ETH_MAX_MTU; err = dpni_set_max_frame_length(priv->mc_io, 0, priv->mc_token, - (u16)DPAA2_ETH_MFL); + DPAA2_ETH_MFL); if (err) { dev_err(dev, "dpni_set_max_frame_length() failed\n"); return err; -- 2.7.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: ks7010: pass 'int' instead of 'bool' to 'hostif_mib_set_request_bool'
On Thu, Jul 12, 2018 at 08:20:16PM +0200, Sergio Paracuellos wrote: > On Thu, Jul 12, 2018 at 2:13 PM, Dan Carpenter > wrote: > > On Thu, Jul 12, 2018 at 06:57:42AM +0200, Sergio Paracuellos wrote: > >> 'hostif_mib_set_request_bool' function receives a bool as value and > >> send the received value with MIB_VALUE_TYPE_BOOL type. There is > >> one case where the value passed is not a boolean one but > >> 'MCAST_FILTER_PROMISC' which is '2'. Pass 'int' instead to avoid > >> the problem. > >> > >> Fixes: 8ce76bff0e6a ("staging: ks7010: add new helpers to achieve > >> mib set request and simplify code") > >> > >> Reported-by: Dan Carpenter > >> Signed-off-by: Sergio Paracuellos > >> --- > >> drivers/staging/ks7010/ks_hostif.c | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/drivers/staging/ks7010/ks_hostif.c > >> b/drivers/staging/ks7010/ks_hostif.c > >> index 0ecffab..6582566 100644 > >> --- a/drivers/staging/ks7010/ks_hostif.c > >> +++ b/drivers/staging/ks7010/ks_hostif.c > >> @@ -1228,7 +1228,7 @@ static inline void hostif_mib_set_request_int(struct > >> ks_wlan_private *priv, > >> > >> static inline void hostif_mib_set_request_bool(struct ks_wlan_private > >> *priv, > >> enum mib_attribute attr, > >> -bool val) > >> +int val) > > > > Huh... This doesn't feel like the right thing. I thought we should > > change the callers to use hostif_mib_set_request_int() instead. > > Yes, I though to call that instead at first moment but I end up in revert to > the > previous behaviour... It's a tricky thing... The choices are: 1) Fix it in the CorrectWay[tm] which we both agree is hostif_mib_set_request_int() 2) Revert to something that looks buggy. But there is a chance it has been tested and works. We would hide the static checker warning which would make the bug harder to fix in the future. 3) Leave the code as-is and wait until someone can test it. At least the static checker warning is there so we will fix it eventually. I feel like we should take option 1 and if no one complains that means either no one is using the driver or it works. Long term that's the best option. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/4] staging: gasket: add SPDX-License-Identifier tag
Use GPL-2.0 based on the license text in each of the files. Remove license "boiler-plate". Signed-off-by: Felix Siegel --- drivers/staging/gasket/apex.h | 1 + drivers/staging/gasket/apex_driver.c | 9 + drivers/staging/gasket/gasket.h| 9 + drivers/staging/gasket/gasket_constants.h | 9 + drivers/staging/gasket/gasket_core.c | 9 + drivers/staging/gasket/gasket_core.h | 9 + drivers/staging/gasket/gasket_interrupt.c | 9 + drivers/staging/gasket/gasket_interrupt.h | 9 + drivers/staging/gasket/gasket_ioctl.c | 9 + drivers/staging/gasket/gasket_ioctl.h | 9 + drivers/staging/gasket/gasket_logging.h| 9 + drivers/staging/gasket/gasket_page_table.c | 9 + drivers/staging/gasket/gasket_page_table.h | 9 + drivers/staging/gasket/gasket_sysfs.c | 9 + drivers/staging/gasket/gasket_sysfs.h | 9 + 15 files changed, 15 insertions(+), 112 deletions(-) diff --git a/drivers/staging/gasket/apex.h b/drivers/staging/gasket/apex.h index f2600aa..1d1f34d 100644 --- a/drivers/staging/gasket/apex.h +++ b/drivers/staging/gasket/apex.h @@ -1,3 +1,4 @@ +/* SPDX-License-Identifier: GPL-2.0 */ /* * Apex kernel-userspace interface definition(s). * diff --git a/drivers/staging/gasket/apex_driver.c b/drivers/staging/gasket/apex_driver.c index 3952567..ffcc59d 100644 --- a/drivers/staging/gasket/apex_driver.c +++ b/drivers/staging/gasket/apex_driver.c @@ -1,15 +1,8 @@ +// SPDX-License-Identifier: GPL-2.0 /* Driver for the Apex chip. * * Copyright (C) 2018 Google, Inc. * - * This software is licensed under the terms of the GNU General Public - * License version 2, as published by the Free Software Foundation, and - * may be copied, distributed, and modified under those terms. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. */ #include diff --git a/drivers/staging/gasket/gasket.h b/drivers/staging/gasket/gasket.h index 593d508..8f45f68 100644 --- a/drivers/staging/gasket/gasket.h +++ b/drivers/staging/gasket/gasket.h @@ -1,15 +1,8 @@ +/* SPDX-License-Identifier: GPL-2.0 */ /* Common Gasket device kernel and user space declarations. * * Copyright (C) 2018 Google, Inc. * - * This software is licensed under the terms of the GNU General Public - * License version 2, as published by the Free Software Foundation, and - * may be copied, distributed, and modified under those terms. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. */ #ifndef __GASKET_H__ #define __GASKET_H__ diff --git a/drivers/staging/gasket/gasket_constants.h b/drivers/staging/gasket/gasket_constants.h index b39e3e3..9a04035 100644 --- a/drivers/staging/gasket/gasket_constants.h +++ b/drivers/staging/gasket/gasket_constants.h @@ -1,13 +1,6 @@ +/* SPDX-License-Identifier: GPL-2.0 */ /* Copyright (C) 2018 Google, Inc. * - * This software is licensed under the terms of the GNU General Public - * License version 2, as published by the Free Software Foundation, and - * may be copied, distributed, and modified under those terms. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. */ #ifndef __GASKET_CONSTANTS_H__ #define __GASKET_CONSTANTS_H__ diff --git a/drivers/staging/gasket/gasket_core.c b/drivers/staging/gasket/gasket_core.c index 45914eb..24e85fb 100644 --- a/drivers/staging/gasket/gasket_core.c +++ b/drivers/staging/gasket/gasket_core.c @@ -1,17 +1,10 @@ +// SPDX-License-Identifier: GPL-2.0 /* Gasket generic driver framework. This file contains the implementation * for the Gasket generic driver framework - the functionality that is common * across Gasket devices. * * Copyright (C) 2018 Google, Inc. * - * This software is licensed under the terms of the GNU General Public - * License version 2, as published by the Free Software Foundation, and - * may be copied, distributed, and modified under those terms. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. */ #include "gasket_core.h" diff --git a/drivers/staging/gasket/gasket_core.h b/drivers/staging/gasket/gasket_core.h index 5d6535a..62bc8fb 100644 --- a/drivers/staging/gasket/gaske
Re: [PATCH 1/4] staging: gasket: add SPDX-License-Identifier tag
On Thu, Jul 12, 2018 at 09:27:12PM +0200, Felix Siegel wrote: > Use GPL-2.0 based on the license text in each of the files. > Remove license "boiler-plate". > > Signed-off-by: Felix Siegel I sent this same patch 1 1/2 days ago :) sorry, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/4] staging: gasket: Move open-curly brace to match kernel code style
Move open open-curly brace to the next line following function definition to match the kernel's coding style Signed-off-by: Felix Siegel --- drivers/staging/gasket/gasket_core.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/staging/gasket/gasket_core.c b/drivers/staging/gasket/gasket_core.c index 24e85fb..d7fdfa1 100644 --- a/drivers/staging/gasket/gasket_core.c +++ b/drivers/staging/gasket/gasket_core.c @@ -1895,7 +1895,8 @@ int gasket_reset_nolock(struct gasket_dev *gasket_dev, uint reset_type) EXPORT_SYMBOL(gasket_reset_nolock); gasket_ioctl_permissions_cb_t gasket_get_ioctl_permissions_cb( - struct gasket_dev *gasket_dev) { + struct gasket_dev *gasket_dev) +{ return gasket_dev->internal_desc->driver_desc->ioctl_permissions_cb; } EXPORT_SYMBOL(gasket_get_ioctl_permissions_cb); -- 2.7.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 3/4] staging: gasket: fix multi line comments style
This patch fixes checkpatch.pl warnings: WARNING: Block comments should align the * on each line Signed-off-by: Felix Siegel --- drivers/staging/gasket/gasket_core.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/staging/gasket/gasket_core.c b/drivers/staging/gasket/gasket_core.c index d7fdfa1..5eeaae7 100644 --- a/drivers/staging/gasket/gasket_core.c +++ b/drivers/staging/gasket/gasket_core.c @@ -1347,9 +1347,9 @@ static bool gasket_mm_get_mapping_addrs( *virt_offset = 0; if (bar_offset + requested_length < range_start) { /* - * If the requested region is completely below the range, - * there is nothing to map. - */ +* If the requested region is completely below the range, +* there is nothing to map. +*/ return false; } else if (bar_offset <= range_start) { /* If the bar offset is below this range's start @@ -1507,7 +1507,7 @@ static enum do_map_region_status do_map_region( * Calculates the offset where the VMA range begins in its containing BAR. * The offset is written into bar_offset on success. * Returns zero on success, anything else on error. -*/ + */ static int gasket_mm_vma_bar_offset( const struct gasket_dev *gasket_dev, const struct vm_area_struct *vma, ulong *bar_offset) -- 2.7.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 4/4] staging: gasket: Use __func__ instead of hardcoded string - Style
Changed logging statements to use %s and __func__ instead of hard coding the function name in a string. Signed-off-by: Felix Siegel --- drivers/staging/gasket/apex_driver.c | 15 --- drivers/staging/gasket/gasket_core.c | 11 ++- drivers/staging/gasket/gasket_ioctl.c | 4 ++-- drivers/staging/gasket/gasket_page_table.c | 13 - 4 files changed, 24 insertions(+), 19 deletions(-) diff --git a/drivers/staging/gasket/apex_driver.c b/drivers/staging/gasket/apex_driver.c index ffcc59d..735d43d 100644 --- a/drivers/staging/gasket/apex_driver.c +++ b/drivers/staging/gasket/apex_driver.c @@ -341,7 +341,7 @@ static int apex_add_dev_cb(struct gasket_dev *gasket_dev) ulong page_table_ready, msix_table_ready; int retries = 0; - gasket_log_error(gasket_dev, "apex_add_dev_cb."); + gasket_log_error(gasket_dev, "%s.", __func__); apex_reset(gasket_dev, 0); @@ -422,8 +422,9 @@ static int apex_device_cleanup(struct gasket_dev *gasket_dev) gasket_log_info( gasket_dev, - "apex_device_cleanup 0x%p hib_error 0x%llx scalar_error " + "%s 0x%p hib_error 0x%llx scalar_error " "0x%llx.", + __func__, gasket_dev, hib_error, scalar_error); if (allow_power_save) @@ -447,13 +448,13 @@ static int apex_reset(struct gasket_dev *gasket_dev, uint type) if (bypass_top_level) return 0; - gasket_log_debug(gasket_dev, "apex_reset."); + gasket_log_debug(gasket_dev, "%s.", __func__); if (!is_gcb_in_reset(gasket_dev)) { /* We are not in reset - toggle the reset bit so as to force * re-init of custom block */ - gasket_log_debug(gasket_dev, "apex_reset: toggle reset."); + gasket_log_debug(gasket_dev, "%s: toggle reset.", __func__); ret = apex_enter_reset(gasket_dev, type); if (ret) @@ -472,7 +473,7 @@ static int apex_enter_reset(struct gasket_dev *gasket_dev, uint type) if (bypass_top_level) return 0; - gasket_log_debug(gasket_dev, "apex_enter_reset."); + gasket_log_debug(gasket_dev, "%s.", __func__); /* * Software reset: @@ -534,7 +535,7 @@ static int apex_quit_reset(struct gasket_dev *gasket_dev, uint type) if (bypass_top_level) return 0; - gasket_log_debug(gasket_dev, "apex_quit_reset."); + gasket_log_debug(gasket_dev, "%s.", __func__); /* * Disable sleep mode: @@ -681,7 +682,7 @@ static long apex_clock_gating(struct gasket_dev *gasket_dev, ulong arg) return -EFAULT; gasket_log_error( - gasket_dev, "apex_clock_gating %llu", ibuf.enable); + gasket_dev, "%s %llu", __func__, ibuf.enable); if (ibuf.enable) { /* Quiesce AXI, gate GCB clock. */ diff --git a/drivers/staging/gasket/gasket_core.c b/drivers/staging/gasket/gasket_core.c index 5eeaae7..b14a956 100644 --- a/drivers/staging/gasket/gasket_core.c +++ b/drivers/staging/gasket/gasket_core.c @@ -932,7 +932,8 @@ static int gasket_enable_dev( } else { gasket_log_error( gasket_dev, - "gasket_enable_dev with no physical device!!"); + "%s with no physical device!!", + __func__); WARN_ON(1); ddev = NULL; } @@ -2100,9 +2101,9 @@ int gasket_wait_sync( if (diff_nanosec > timeout_ns) { gasket_log_error( gasket_dev, - "gasket_wait_sync timeout: reg %llx count %x " + "%s timeout: reg %llx count %x " "dma %lld ns\n", - offset, count, diff_nanosec); + __func__, offset, count, diff_nanosec); return -1; } reg = gasket_dev_read_64(gasket_dev, bar, offset); @@ -2141,8 +2142,8 @@ int gasket_wait_with_reschedule( if (retries == max_retries) { gasket_log_error( gasket_dev, - "gasket_wait_with_reschedule timeout: reg %llx timeout (%llu ms)", - offset, max_retries * delay_ms); + "%s timeout: reg %llx timeout (%llu ms)", + __func__, offset, max_retries * delay_ms); return -EINVAL; } return 0; diff --git a/drivers/staging/gasket/gasket_ioctl.c b/drivers/staging/gasket/gasket_ioctl.c index 832beed..738c56f 100644 --- a/drivers/staging/gasket/gasket_ioctl.c +++ b/
Re: [PATCH 4/4] staging: gasket: Use __func__ instead of hardcoded string - Style
On Thu, Jul 12, 2018 at 09:27:15PM +0200, Felix Siegel wrote: > Changed logging statements to use %s and __func__ instead of hard coding > the function name in a string. > > Signed-off-by: Felix Siegel > --- > drivers/staging/gasket/apex_driver.c | 15 --- > drivers/staging/gasket/gasket_core.c | 11 ++- > drivers/staging/gasket/gasket_ioctl.c | 4 ++-- > drivers/staging/gasket/gasket_page_table.c | 13 - > 4 files changed, 24 insertions(+), 19 deletions(-) > > diff --git a/drivers/staging/gasket/apex_driver.c > b/drivers/staging/gasket/apex_driver.c > index ffcc59d..735d43d 100644 > --- a/drivers/staging/gasket/apex_driver.c > +++ b/drivers/staging/gasket/apex_driver.c > @@ -341,7 +341,7 @@ static int apex_add_dev_cb(struct gasket_dev *gasket_dev) > ulong page_table_ready, msix_table_ready; > int retries = 0; > > - gasket_log_error(gasket_dev, "apex_add_dev_cb."); > + gasket_log_error(gasket_dev, "%s.", __func__); Function calls that do nothing but log "Look at this function I just entered/exited!" need to just be deleted entirely, as ftrace should be used instead. Care to do that here, and then send a patch for the remaining messages that do need __func__? thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 3/4] staging: gasket: fix multi line comments style
On Thu, Jul 12, 2018 at 09:27:14PM +0200, Felix Siegel wrote: > This patch fixes checkpatch.pl warnings: > > WARNING: Block comments should align the * on each line > Signed-off-by: Felix Siegel So close, I need a blank line before the signed-of-by line :( I'll go edit it by hand, but be more careful next time please... thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/4] staging: gasket: add SPDX-License-Identifier tag
On Thu, 12 Jul 2018 21:42:49 +0200 Greg Kroah-Hartman wrote: > On Thu, Jul 12, 2018 at 09:27:12PM +0200, Felix Siegel wrote: > > Use GPL-2.0 based on the license text in each of the files. > > Remove license "boiler-plate". > > > > Signed-off-by: Felix Siegel > > I sent this same patch 1 1/2 days ago :) > > sorry, > > greg k-h My bad, I should have checked before sending it. Regards, Felix ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 3/4] staging: gasket: fix multi line comments style
On Thu, 12 Jul 2018 21:53:54 +0200 Greg Kroah-Hartman wrote: > On Thu, Jul 12, 2018 at 09:27:14PM +0200, Felix Siegel wrote: > > This patch fixes checkpatch.pl warnings: > > > > WARNING: Block comments should align the * on each line > > Signed-off-by: Felix Siegel > > So close, I need a blank line before the signed-of-by line :( > > I'll go edit it by hand, but be more careful next time please... > > thanks, > > greg k-h Sorry, I'll look out for it next time. Strange though, they were all added with format-patch -s. Regards, Felix ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/2] staging: bcm2835-audio: Check if workqueue allocation failed
Currently, if allocating a workqueue fails, the driver will probe successfully but it will silently do nothing, which is rather silly. So instead bail out with -ENOMEM in bcm2835_audio_open() if alloc_workqueue() fails, and remove the now pointless checks for a NULL workqueue. While at it, get rid of the rather pointless one-line function my_workqueue_init(). Signed-off-by: Tuomas Tynkkynen --- .../vc04_services/bcm2835-audio/bcm2835-vchiq.c| 111 ++--- 1 file changed, 50 insertions(+), 61 deletions(-) diff --git a/drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c b/drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c index a4a48f31f1a3..85ed807bb873 100644 --- a/drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c +++ b/drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c @@ -118,44 +118,40 @@ static void my_wq_function(struct work_struct *work) int bcm2835_audio_start(struct bcm2835_alsa_stream *alsa_stream) { - if (alsa_stream->my_wq) { - struct bcm2835_audio_work *work; - - work = kmalloc(sizeof(*work), GFP_ATOMIC); - /*--- Queue some work (item 1) ---*/ - if (!work) { - LOG_ERR(" .. Error: NULL work kmalloc\n"); - return -ENOMEM; - } - INIT_WORK(&work->my_work, my_wq_function); - work->alsa_stream = alsa_stream; - work->cmd = BCM2835_AUDIO_START; - if (!queue_work(alsa_stream->my_wq, &work->my_work)) { - kfree(work); - return -EBUSY; - } + struct bcm2835_audio_work *work; + + work = kmalloc(sizeof(*work), GFP_ATOMIC); + /*--- Queue some work (item 1) ---*/ + if (!work) { + LOG_ERR(" .. Error: NULL work kmalloc\n"); + return -ENOMEM; + } + INIT_WORK(&work->my_work, my_wq_function); + work->alsa_stream = alsa_stream; + work->cmd = BCM2835_AUDIO_START; + if (!queue_work(alsa_stream->my_wq, &work->my_work)) { + kfree(work); + return -EBUSY; } return 0; } int bcm2835_audio_stop(struct bcm2835_alsa_stream *alsa_stream) { - if (alsa_stream->my_wq) { - struct bcm2835_audio_work *work; - - work = kmalloc(sizeof(*work), GFP_ATOMIC); - /*--- Queue some work (item 1) ---*/ - if (!work) { - LOG_ERR(" .. Error: NULL work kmalloc\n"); - return -ENOMEM; - } - INIT_WORK(&work->my_work, my_wq_function); - work->alsa_stream = alsa_stream; - work->cmd = BCM2835_AUDIO_STOP; - if (!queue_work(alsa_stream->my_wq, &work->my_work)) { - kfree(work); - return -EBUSY; - } + struct bcm2835_audio_work *work; + + work = kmalloc(sizeof(*work), GFP_ATOMIC); + /*--- Queue some work (item 1) ---*/ + if (!work) { + LOG_ERR(" .. Error: NULL work kmalloc\n"); + return -ENOMEM; + } + INIT_WORK(&work->my_work, my_wq_function); + work->alsa_stream = alsa_stream; + work->cmd = BCM2835_AUDIO_STOP; + if (!queue_work(alsa_stream->my_wq, &work->my_work)) { + kfree(work); + return -EBUSY; } return 0; } @@ -163,40 +159,31 @@ int bcm2835_audio_stop(struct bcm2835_alsa_stream *alsa_stream) int bcm2835_audio_write(struct bcm2835_alsa_stream *alsa_stream, unsigned int count, void *src) { - if (alsa_stream->my_wq) { - struct bcm2835_audio_work *work; - - work = kmalloc(sizeof(*work), GFP_ATOMIC); - /*--- Queue some work (item 1) ---*/ - if (!work) { - LOG_ERR(" .. Error: NULL work kmalloc\n"); - return -ENOMEM; - } - INIT_WORK(&work->my_work, my_wq_function); - work->alsa_stream = alsa_stream; - work->cmd = BCM2835_AUDIO_WRITE; - work->src = src; - work->count = count; - if (!queue_work(alsa_stream->my_wq, &work->my_work)) { - kfree(work); - return -EBUSY; - } + struct bcm2835_audio_work *work; + + work = kmalloc(sizeof(*work), GFP_ATOMIC); + /*--- Queue some work (item 1) ---*/ + if (!work) { + LOG_ERR(" .. Error: NULL work kmalloc\n"); + return -ENOMEM; + } + INIT_WORK(&work->my_work, my_wq_function); + work->alsa_stream = alsa_stream; + work->cmd = BCM2835_AUDIO_WRITE; + work->src = src; + work->count = count; + if (!queue_work(alsa_stream->my_wq, &work->my_work)) { + kfree(work); +
[PATCH 2/2] staging: bcm2835-audio: Don't leak workqueue if open fails
Currently, if bcm2835_audio_open() fails partway, the allocated workqueue is leaked. Avoid that. While at it, propagate the return value of bcm2835_audio_open_connection() on failure instead of returning -1. Signed-off-by: Tuomas Tynkkynen --- .../staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c | 16 ++-- 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c b/drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c index 85ed807bb873..779c1e993b55 100644 --- a/drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c +++ b/drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c @@ -416,16 +416,16 @@ int bcm2835_audio_open(struct bcm2835_alsa_stream *alsa_stream) return -ENOMEM; ret = bcm2835_audio_open_connection(alsa_stream); - if (ret) { - ret = -1; - goto exit; - } + if (ret) + goto free_wq; + instance = alsa_stream->instance; LOG_DBG(" instance (%p)\n", instance); if (mutex_lock_interruptible(&instance->vchi_mutex)) { LOG_DBG("Interrupted whilst waiting for lock on (%d)\n", instance->num_connections); - return -EINTR; + ret = -EINTR; + goto free_wq; } vchi_service_use(instance->vchi_handle[0]); @@ -448,7 +448,11 @@ int bcm2835_audio_open(struct bcm2835_alsa_stream *alsa_stream) unlock: vchi_service_release(instance->vchi_handle[0]); mutex_unlock(&instance->vchi_mutex); -exit: + +free_wq: + if (ret) + destroy_workqueue(alsa_stream->my_wq); + return ret; } -- 2.16.3 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: speakup: fix wraparound in uaccess length check
On Fri, Jul 13, 2018 at 12:29:36AM +0200, Jann Horn wrote: > From: Samuel Thibault > > From: Samuel Thibault > > If softsynthx_read() is called with `count < 3`, `count - 3` wraps, causing > the loop to copy as much data as available to the provided buffer. If > softsynthx_read() is invoked through sys_splice(), this causes an > unbounded kernel write; but even when userspace just reads from it > normally, a small size could cause userspace crashes. Or you could try this (completely untested, though): diff --git a/drivers/staging/speakup/speakup_soft.c b/drivers/staging/speakup/speakup_soft.c index a61bc41b82d7..198936ed0b54 100644 --- a/drivers/staging/speakup/speakup_soft.c +++ b/drivers/staging/speakup/speakup_soft.c @@ -192,13 +192,10 @@ static int softsynth_close(struct inode *inode, struct file *fp) return 0; } -static ssize_t softsynthx_read(struct file *fp, char __user *buf, size_t count, - loff_t *pos, int unicode) +static ssize_t softsynthx_read_iter(struct kiocb *iocb, struct iov_iter *to, int unicode) { int chars_sent = 0; - char __user *cp; char *init; - u16 ch; int empty; unsigned long flags; DEFINE_WAIT(wait); @@ -211,7 +208,7 @@ static ssize_t softsynthx_read(struct file *fp, char __user *buf, size_t count, if (!synth_buffer_empty() || speakup_info.flushing) break; spin_unlock_irqrestore(&speakup_info.spinlock, flags); - if (fp->f_flags & O_NONBLOCK) { + if (iocb->ki_flags & IOCB_NOWAIT) { finish_wait(&speakup_event, &wait); return -EAGAIN; } @@ -224,11 +221,14 @@ static ssize_t softsynthx_read(struct file *fp, char __user *buf, size_t count, } finish_wait(&speakup_event, &wait); - cp = buf; init = get_initstring(); /* Keep 3 bytes available for a 16bit UTF-8-encoded character */ - while (chars_sent <= count - 3) { + while (iov_iter_count(to)) { + u_char s[3]; + int l, n; + u16 ch; + if (speakup_info.flushing) { speakup_info.flushing = 0; ch = '\x18'; @@ -244,60 +244,47 @@ static ssize_t softsynthx_read(struct file *fp, char __user *buf, size_t count, spin_unlock_irqrestore(&speakup_info.spinlock, flags); if ((!unicode && ch < 0x100) || (unicode && ch < 0x80)) { - u_char c = ch; - - if (copy_to_user(cp, &c, 1)) - return -EFAULT; - - chars_sent++; - cp++; + s[0] = ch; + l = 1; } else if (unicode && ch < 0x800) { - u_char s[2] = { - 0xc0 | (ch >> 6), - 0x80 | (ch & 0x3f) - }; - - if (copy_to_user(cp, s, sizeof(s))) - return -EFAULT; - - chars_sent += sizeof(s); - cp += sizeof(s); + s[0] = 0xc0 | (ch >> 6); + s[1] = 0x80 | (ch & 0x3f); + l = 2; } else if (unicode) { - u_char s[3] = { - 0xe0 | (ch >> 12), - 0x80 | ((ch >> 6) & 0x3f), - 0x80 | (ch & 0x3f) - }; - - if (copy_to_user(cp, s, sizeof(s))) - return -EFAULT; - - chars_sent += sizeof(s); - cp += sizeof(s); + s[0] = 0xe0 | (ch >> 12); + s[1] = 0x80 | ((ch >> 6) & 0x3f); + s[2] = 0x80 | (ch & 0x3f); + l = 3; } - + n = copy_to_iter(s, l, to); + if (n != l) { + iov_iter_revert(to, n); + spin_lock_irqsave(&speakup_info.spinlock, flags); + break; + } + chars_sent += l; spin_lock_irqsave(&speakup_info.spinlock, flags); } - *pos += chars_sent; + iocb->ki_pos += chars_sent; empty = synth_buffer_empty(); spin_unlock_irqrestore(&speakup_info.spinlock, flags); if (empty) { speakup_start_ttys(); - *pos = 0; + iocb->ki_pos = 0; } return chars_sent; } -static ssize_t softsynth_read(struct file *fp, char __user *buf, size_t count, +static ssize_t softsynth_read_iter(struct kiocb *iocb, struct iov_iter *to) loff_t *pos) { - return softsynthx_read(fp, buf, count
Re: [PATCH 4/4] staging: gasket: Use __func__ instead of hardcoded string - Style
> Function calls that do nothing but log "Look at this function I just > entered/exited!" need to just be deleted entirely, as ftrace should be > used instead. > > Care to do that here, and then send a patch for the remaining messages > that do need __func__? Sure. I hope this is how you meant it. I kept the calls that print more than just the name of the function. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 1/2] staging: gasket: remove "function entered" log messages
Remove log messages that solely print the function's name everytime it is called. Signed-off-by: Felix Siegel --- drivers/staging/gasket/apex_driver.c | 8 1 file changed, 8 deletions(-) diff --git a/drivers/staging/gasket/apex_driver.c b/drivers/staging/gasket/apex_driver.c index 3952567..9218ad7 100644 --- a/drivers/staging/gasket/apex_driver.c +++ b/drivers/staging/gasket/apex_driver.c @@ -348,8 +348,6 @@ static int apex_add_dev_cb(struct gasket_dev *gasket_dev) ulong page_table_ready, msix_table_ready; int retries = 0; - gasket_log_error(gasket_dev, "apex_add_dev_cb."); - apex_reset(gasket_dev, 0); while (retries < APEX_RESET_RETRY) { @@ -454,8 +452,6 @@ static int apex_reset(struct gasket_dev *gasket_dev, uint type) if (bypass_top_level) return 0; - gasket_log_debug(gasket_dev, "apex_reset."); - if (!is_gcb_in_reset(gasket_dev)) { /* We are not in reset - toggle the reset bit so as to force * re-init of custom block @@ -479,8 +475,6 @@ static int apex_enter_reset(struct gasket_dev *gasket_dev, uint type) if (bypass_top_level) return 0; - gasket_log_debug(gasket_dev, "apex_enter_reset."); - /* * Software reset: * Enable sleep mode @@ -541,8 +535,6 @@ static int apex_quit_reset(struct gasket_dev *gasket_dev, uint type) if (bypass_top_level) return 0; - gasket_log_debug(gasket_dev, "apex_quit_reset."); - /* * Disable sleep mode: * - Disable GCB memory shut down: -- 2.7.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 2/2] staging: gasket: Use __func__ instead of hardcoded string - Style
Changed logging statements to use %s and __func__ instead of hard coding the function name in a string. Signed-off-by: Felix Siegel --- drivers/staging/gasket/apex_driver.c | 7 --- drivers/staging/gasket/gasket_core.c | 8 +--- drivers/staging/gasket/gasket_ioctl.c | 3 ++- drivers/staging/gasket/gasket_page_table.c | 13 - 4 files changed, 19 insertions(+), 12 deletions(-) diff --git a/drivers/staging/gasket/apex_driver.c b/drivers/staging/gasket/apex_driver.c index 9218ad7..ee91137 100644 --- a/drivers/staging/gasket/apex_driver.c +++ b/drivers/staging/gasket/apex_driver.c @@ -427,8 +427,9 @@ static int apex_device_cleanup(struct gasket_dev *gasket_dev) gasket_log_info( gasket_dev, - "apex_device_cleanup 0x%p hib_error 0x%llx scalar_error " + "%s 0x%p hib_error 0x%llx scalar_error " "0x%llx.", + __func__, gasket_dev, hib_error, scalar_error); if (allow_power_save) @@ -456,7 +457,7 @@ static int apex_reset(struct gasket_dev *gasket_dev, uint type) /* We are not in reset - toggle the reset bit so as to force * re-init of custom block */ - gasket_log_debug(gasket_dev, "apex_reset: toggle reset."); + gasket_log_debug(gasket_dev, "%s: toggle reset.", __func__); ret = apex_enter_reset(gasket_dev, type); if (ret) @@ -680,7 +681,7 @@ static long apex_clock_gating(struct gasket_dev *gasket_dev, ulong arg) return -EFAULT; gasket_log_error( - gasket_dev, "apex_clock_gating %llu", ibuf.enable); + gasket_dev, "%s %llu", __func__, ibuf.enable); if (ibuf.enable) { /* Quiesce AXI, gate GCB clock. */ diff --git a/drivers/staging/gasket/gasket_core.c b/drivers/staging/gasket/gasket_core.c index fa3cd46..f96ca55 100644 --- a/drivers/staging/gasket/gasket_core.c +++ b/drivers/staging/gasket/gasket_core.c @@ -939,7 +939,7 @@ static int gasket_enable_dev( } else { gasket_log_error( gasket_dev, - "gasket_enable_dev with no physical device!!"); + "%s with no physical device!!", __func__); WARN_ON(1); ddev = NULL; } @@ -2107,8 +2107,9 @@ int gasket_wait_sync( if (diff_nanosec > timeout_ns) { gasket_log_error( gasket_dev, - "gasket_wait_sync timeout: reg %llx count %x " + "%s timeout: reg %llx count %x " "dma %lld ns\n", + __func__, offset, count, diff_nanosec); return -1; } @@ -2148,7 +2149,8 @@ int gasket_wait_with_reschedule( if (retries == max_retries) { gasket_log_error( gasket_dev, - "gasket_wait_with_reschedule timeout: reg %llx timeout (%llu ms)", + "%s timeout: reg %llx timeout (%llu ms)", + __func__, offset, max_retries * delay_ms); return -EINVAL; } diff --git a/drivers/staging/gasket/gasket_ioctl.c b/drivers/staging/gasket/gasket_ioctl.c index 4758083..417f136 100644 --- a/drivers/staging/gasket/gasket_ioctl.c +++ b/drivers/staging/gasket/gasket_ioctl.c @@ -187,7 +187,8 @@ static uint gasket_ioctl_check_permissions(struct file *filp, uint cmd) alive = (gasket_dev->status == GASKET_STATUS_ALIVE); if (!alive) { gasket_nodev_error( - "gasket_ioctl_check_permissions alive %d status %d.", + "%s alive %d status %d.", + __func__, alive, gasket_dev->status); } diff --git a/drivers/staging/gasket/gasket_page_table.c b/drivers/staging/gasket/gasket_page_table.c index 5d3d33c..961db41 100644 --- a/drivers/staging/gasket/gasket_page_table.c +++ b/drivers/staging/gasket/gasket_page_table.c @@ -452,8 +452,9 @@ int gasket_page_table_map( mutex_unlock(&pg_tbl->mutex); gasket_nodev_debug( - "gasket_page_table_map done: ha %llx daddr %llx num %d, " + "%s done: ha %llx daddr %llx num %d, " "ret %d\n", + __func__, (unsigned long long)host_addr, (unsigned long long)dev_addr, num_pages, ret); return ret; @@ -876,7 +877,7 @@ static int gasket_perform_mapping( for (i = 0; i < num_pages; i++) { page_addr = host_addr + i * PAGE_SIZE; offset = page_addr
RE: Will the name of hyperv_clocksource_tsc_page or hyperv_clocksource pages change?
Hello, Even after asking again, I never did hear from the customer why it matters if the name changes. The case is being closed, and I am assuming that the information you gave (that I passed onto the customer) was sufficient to answer the customer's question. Thank you Greg, Vitaly, and Peter for helping answer this question. エアー・アルマ Professional Direct Delivery Manager Email: v-ale...@microsoft.com Microsoft Azure Professional Direct Services 日本 +81 1-2051-4100(無料) 日本 +81 3-6743-9670(有料) 電子メール: pdaz...@microsoft.com -Original Message- From: Greg KH Sent: Friday, June 22, 2018 10:51 PM To: Peter Zijlstra Cc: Alma Eyre (Sonata Software North America) ; Haiyang Zhang ; de...@linuxdriverproject.org; Vitaly Kuznetsov ; Linus Torvalds ; Thomas Gleixner Subject: Re: Will the name of hyperv_clocksource_tsc_page or hyperv_clocksource pages change? On Fri, Jun 22, 2018 at 10:22:28AM +0200, Peter Zijlstra wrote: > On Fri, Jun 22, 2018 at 03:17:25AM +, Alma Eyre (Sonata Software North > America) wrote: > > Hello, > > > > This is Alma supporting Azure for Japanese customers. I had a > > question from a customer that I could not find the answers for. I > > saw this > > github(https://na01.safelinks.protection.outlook.com/?url=https%3A%2 > > F%2Fgithub.com%2Ftorvalds%2Flinux%2Fcommit%2F88c9281a9fba67636ab26c1 > > fd6afbc78a632374f&data=02%7C01%7Cv-aleyre%40microsoft.com%7Cfd2c > > bcc41da54a65be1908d5d8cd635c%7C72f988bf86f141af91ab2d7cd011db47%7C1% > > 7C0%7C636653299021462729&sdata=B0KCAukioytYR0RlTPr2n3KVCrKzkRem2 > > ir1aBiXXoA%3D&reserved=0) page, and I was wondering if someone > > on this list might be able to answer the question. > > > > Will the name of hyperv_clocksource_tsc_page or hyperv_clocksource > > pages change? > > https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithu > b.com%2Ftorvalds%2Flinux%2Fblob%2Fe7aa8c2eb11ba69b1b69099c3c7bd6be3087 > b0ba%2FDocumentation%2Fprocess%2Fstable-api-nonsense.rst&data=02%7 > C01%7Cv-aleyre%40microsoft.com%7Cfd2cbcc41da54a65be1908d5d8cd635c%7C72 > f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636653299021462729&sdata= > URuIH1hhfWowC4yfkZxZB8Gg9%2Fo6rEkzufMOUSgjJug%3D&reserved=0 Or better yet, in a pretty html format: https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.kernel.org%2Fdoc%2Fhtml%2Flatest%2Fprocess%2Fstable-api-nonsense.html&data=02%7C01%7Cv-aleyre%40microsoft.com%7Cfd2cbcc41da54a65be1908d5d8cd635c%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636653299021462729&sdata=UBkvgJz9XcURXiTPbV1CImpK9DsnbK9HTgFwZCKjcJU%3D&reserved=0 But, this is a name of a clocksource, not really an internal kernel api. Alma, what external tool depends on the specific name of a kernel clock? Why would it matter what the name of it is? thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: ks7010: pass 'int' instead of 'bool' to 'hostif_mib_set_request_bool'
On Thu, Jul 12, 2018 at 10:11:25PM +0300, Dan Carpenter wrote: > On Thu, Jul 12, 2018 at 08:20:16PM +0200, Sergio Paracuellos wrote: > > On Thu, Jul 12, 2018 at 2:13 PM, Dan Carpenter > > wrote: > > > On Thu, Jul 12, 2018 at 06:57:42AM +0200, Sergio Paracuellos wrote: > > >> 'hostif_mib_set_request_bool' function receives a bool as value and > > >> send the received value with MIB_VALUE_TYPE_BOOL type. There is > > >> one case where the value passed is not a boolean one but > > >> 'MCAST_FILTER_PROMISC' which is '2'. Pass 'int' instead to avoid > > >> the problem. > > >> > > >> Fixes: 8ce76bff0e6a ("staging: ks7010: add new helpers to achieve > > >> mib set request and simplify code") > > >> > > >> Reported-by: Dan Carpenter > > >> Signed-off-by: Sergio Paracuellos > > >> --- > > >> drivers/staging/ks7010/ks_hostif.c | 2 +- > > >> 1 file changed, 1 insertion(+), 1 deletion(-) > > >> > > >> diff --git a/drivers/staging/ks7010/ks_hostif.c > > >> b/drivers/staging/ks7010/ks_hostif.c > > >> index 0ecffab..6582566 100644 > > >> --- a/drivers/staging/ks7010/ks_hostif.c > > >> +++ b/drivers/staging/ks7010/ks_hostif.c > > >> @@ -1228,7 +1228,7 @@ static inline void > > >> hostif_mib_set_request_int(struct ks_wlan_private *priv, > > >> > > >> static inline void hostif_mib_set_request_bool(struct ks_wlan_private > > >> *priv, > > >> enum mib_attribute attr, > > >> -bool val) > > >> +int val) > > > > > > Huh... This doesn't feel like the right thing. I thought we should > > > change the callers to use hostif_mib_set_request_int() instead. > > > > Yes, I though to call that instead at first moment but I end up in revert > > to the > > previous behaviour... > > It's a tricky thing... > > The choices are: > > 1) Fix it in the CorrectWay[tm] which we both agree is >hostif_mib_set_request_int() > > 2) Revert to something that looks buggy. But there is a chance it has >been tested and works. We would hide the static checker warning >which would make the bug harder to fix in the future. > > 3) Leave the code as-is and wait until someone can test it. At least >the static checker warning is there so we will fix it eventually. > > I feel like we should take option 1 and if no one complains that means > either no one is using the driver or it works. Long term that's the > best option. Agreed. I'll send a v2 patch calling 'hostif_mib_set_request_int'. > > regards, > dan carpenter > Best regards, Sergio Paracuellos ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2] staging: ks7010: call 'hostif_mib_set_request_int' instead of 'hostif_mib_set_request_bool'
'hostif_mib_set_request_bool' function receives a bool as value and send the received value with MIB_VALUE_TYPE_BOOL type. There is one case where the value passed is not a boolean one but 'MCAST_FILTER_PROMISC' which is '2'. Call hostif_mib_set_request_int instead. This changes original code behaviour but seems to be the right way to do this. Fixes: 8ce76bff0e6a ("staging: ks7010: add new helpers to achieve mib set request and simplify code") Reported-by: Dan Carpenter Signed-off-by: Sergio Paracuellos --- Changes in v2: - Rename commit message. Previous patch was: staging: ks7010: pass 'int' instead of 'bool' to 'hostif_mib_set_request_bool' - Call 'hostif_mib_set_request_int' for this case instead of change hostif_mib_set_request_bool parameter type. This has been reported as a bug here: http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/2018-July/123058.html drivers/staging/ks7010/ks_hostif.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/staging/ks7010/ks_hostif.c b/drivers/staging/ks7010/ks_hostif.c index a85975f..94bee25 100644 --- a/drivers/staging/ks7010/ks_hostif.c +++ b/drivers/staging/ks7010/ks_hostif.c @@ -1840,8 +1840,8 @@ void hostif_sme_multicast_set(struct ks_wlan_private *priv) memset(set_address, 0, NIC_MAX_MCAST_LIST * ETH_ALEN); if (dev->flags & IFF_PROMISC) { - hostif_mib_set_request_bool(priv, LOCAL_MULTICAST_FILTER, - MCAST_FILTER_PROMISC); + hostif_mib_set_request_int(priv, LOCAL_MULTICAST_FILTER, + MCAST_FILTER_PROMISC); goto spin_unlock; } -- 2.7.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 1/2] staging: mt7621-pci: use generic kernel pci subsystem read and write
On Thu, Jul 12, 2018 at 09:28:15PM +0300, Dan Carpenter wrote: > On Thu, Jul 12, 2018 at 08:15:00PM +0200, Sergio Paracuellos wrote: > > map_bus callback is called before every .read/.write operation. > > Implement it and change custom read write operations for the > > pci subsystem generics. Make the probe function to assign data > > for controller data and get pci register base from device tree. > > > > Signed-off-by: Sergio Paracuellos > > --- > > drivers/staging/mt7621-pci/pci-mt7621.c | 99 > > +++-- > > 1 file changed, 95 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/staging/mt7621-pci/pci-mt7621.c > > b/drivers/staging/mt7621-pci/pci-mt7621.c > > index 4f56840..6187d41 100644 > > --- a/drivers/staging/mt7621-pci/pci-mt7621.c > > +++ b/drivers/staging/mt7621-pci/pci-mt7621.c > > @@ -52,6 +52,9 @@ > > #include > > #include > > #include > > +#include > > +#include > > +#include > > #include > > > > #include > > @@ -177,6 +180,32 @@ static int pcie_link_status = 0; > > #define PCI_ACCESS_WRITE_2 4 > > #define PCI_ACCESS_WRITE_4 5 > > > > +/** > > + * struct mt7621_pcie_port - PCIe port information > > + * @base: IO mapped register base > > + * @list: port list > > + * @pcie: pointer to PCIe host info > > + * @reset: pointer to port reset control > > + */ > > +struct mt7621_pcie_port { > > + void __iomem *base; > > + struct list_head list; > > + struct mt7621_pcie *pcie; > > + struct reset_control *reset; > > +}; > > + > > +/** > > + * struct mt7621_pcie - PCIe host information > > + * @base: IO Mapped Register Base > > + * @dev: Pointer to PCIe device > > + * @ports: pointer to PCIe port information > > + */ > > +struct mt7621_pcie { > > + void __iomem *base; > > + struct device *dev; > > + struct list_head ports; > > +}; > > + > > static inline u32 mt7621_pci_get_cfgaddr(unsigned int bus, unsigned int > > slot, > > unsigned int func, unsigned int where) > > { > > @@ -296,15 +325,27 @@ pci_config_write(struct pci_bus *bus, unsigned int > > devfn, int where, int size, u > > } > > } > > > > +static void __iomem *mt7621_pcie_map_bus(struct pci_bus *bus, > > +unsigned int devfn, int where) > > +{ > > + struct mt7621_pcie *pcie = bus->sysdata; > > + u32 address = mt7621_pci_get_cfgaddr(bus->number, PCI_SLOT(devfn), > > +PCI_FUNC(devfn), where); > > + > > + writel(address, pcie->base + RALINK_PCI_CONFIG_ADDR); > > + > > + return pcie->base + RALINK_PCI_CONFIG_DATA_VIRTUAL_REG + (where & 3); > > +} > > + > > struct pci_ops mt7621_pci_ops = { > > - .read = pci_config_read, > > - .write = pci_config_write, > > + .map_bus= mt7621_pcie_map_bus, > > + .read = pci_generic_config_read, > > + .write = pci_generic_config_write, > > }; > > > > static struct resource mt7621_res_pci_mem1; > > static struct resource mt7621_res_pci_io1; > > static struct pci_controller mt7621_controller = { > > - .pci_ops= &mt7621_pci_ops, > > .mem_resource = &mt7621_res_pci_mem1, > > .io_resource= &mt7621_res_pci_io1, > > }; > > @@ -479,10 +520,61 @@ void setup_cm_memory_region(struct resource > > *mem_resource) > > } > > } > > > > +static int mt7621_pcie_parse_dt(struct mt7621_pcie *pcie) > > +{ > > + struct device *dev = pcie->dev; > > + struct device_node *node = dev->of_node; > > + struct resource regs; > > + const char *type; > > + int err; > > + > > + type = of_get_property(node, "device_type", NULL); > > + if (!type || strcmp(type, "pci") != 0) { > > + dev_err(dev, "invalid \"device_type\" %s\n", type); > > + return -EINVAL; > > + } > > + > > + err = of_address_to_resource(node, 0, ®s); > > + if (err) { > > + dev_err(dev, "missing \"reg\" property\n"); > > + return err; > > + } > > + > > + pcie->base = devm_pci_remap_cfg_resource(dev, ®s); > > + if (IS_ERR(pcie->base)) > > + return PTR_ERR(pcie->base); > > + > > + return 0; > > +} > > + > > static int mt7621_pci_probe(struct platform_device *pdev) > > { > > + struct device *dev = &pdev->dev; > > + struct mt7621_pcie *pcie; > > + struct pci_host_bridge *bridge; > > + int err; > > unsigned long val = 0; > > > > + bridge = devm_pci_alloc_host_bridge(dev, sizeof(*pcie)); > > + if (!bridge) > > + return -ENODEV; > > + > > + pcie = pci_host_bridge_priv(bridge); > > + pcie->dev = dev; > > + INIT_LIST_HEAD(&pcie->ports); > > + > > + err = mt7621_pcie_parse_dt(pcie); > > + if (err) { > > + dev_err(dev, "Parsing DT failed\n"); > > We need to free bridge on this error path. I think is ok as it is. 'devm_pci_alloc_host_bridge' set internally 'devm_pci_release_host_bridge_dev' as 'bridge->dev.release' callback to be called. > > > + return err; > > + }
[PATCH] Drivers: hv: vmbus: Reset the channel callback in vmbus_onoffer_rescind()
Before setting channel->rescind in vmbus_rescind_cleanup(), we should make sure the channel callback won't run any more, otherwise a high-level driver like pci_hyperv, which may be infinitely waiting for the host VSP's response and notices the channel has been rescinded, can't safely give up: e.g., in hv_pci_protocol_negotiation() -> wait_for_response(), it's unsafe to exit from wait_for_response() and proceed with the on-stack variable "comp_pkt" popped. The issue was originally spotted by Michael Kelley . In vmbus_close_internal(), the patch also minimizes the range protected by disabling/enabling channel->callback_event: we don't really need that for the whole function. Signed-off-by: Dexuan Cui Cc: sta...@vger.kernel.org Cc: K. Y. Srinivasan Cc: Stephen Hemminger Cc: Michael Kelley --- drivers/hv/channel.c | 40 drivers/hv/channel_mgmt.c | 6 ++ include/linux/hyperv.h| 2 ++ 3 files changed, 32 insertions(+), 16 deletions(-) diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c index ba0a092..c394922 100644 --- a/drivers/hv/channel.c +++ b/drivers/hv/channel.c @@ -558,11 +558,8 @@ static void reset_channel_cb(void *arg) channel->onchannel_callback = NULL; } -static int vmbus_close_internal(struct vmbus_channel *channel) +void vmbus_reset_channel_cb(struct vmbus_channel *channel) { - struct vmbus_channel_close_channel *msg; - int ret; - /* * vmbus_on_event(), running in the per-channel tasklet, can race * with vmbus_close_internal() in the case of SMP guest, e.g., when @@ -572,6 +569,29 @@ static int vmbus_close_internal(struct vmbus_channel *channel) */ tasklet_disable(&channel->callback_event); + channel->sc_creation_callback = NULL; + + /* Stop the callback asap */ + if (channel->target_cpu != get_cpu()) { + put_cpu(); + smp_call_function_single(channel->target_cpu, reset_channel_cb, +channel, true); + } else { + reset_channel_cb(channel); + put_cpu(); + } + + /* Re-enable tasklet for use on re-open */ + tasklet_enable(&channel->callback_event); +} + +static int vmbus_close_internal(struct vmbus_channel *channel) +{ + struct vmbus_channel_close_channel *msg; + int ret; + + vmbus_reset_channel_cb(channel); + /* * In case a device driver's probe() fails (e.g., * util_probe() -> vmbus_open() returns -ENOMEM) and the device is @@ -585,16 +605,6 @@ static int vmbus_close_internal(struct vmbus_channel *channel) } channel->state = CHANNEL_OPEN_STATE; - channel->sc_creation_callback = NULL; - /* Stop callback and cancel the timer asap */ - if (channel->target_cpu != get_cpu()) { - put_cpu(); - smp_call_function_single(channel->target_cpu, reset_channel_cb, -channel, true); - } else { - reset_channel_cb(channel); - put_cpu(); - } /* Send a closing message */ @@ -639,8 +649,6 @@ static int vmbus_close_internal(struct vmbus_channel *channel) get_order(channel->ringbuffer_pagecount * PAGE_SIZE)); out: - /* re-enable tasklet for use on re-open */ - tasklet_enable(&channel->callback_event); return ret; } diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c index ecc2bd2..9536b93 100644 --- a/drivers/hv/channel_mgmt.c +++ b/drivers/hv/channel_mgmt.c @@ -895,6 +895,12 @@ static void vmbus_onoffer_rescind(struct vmbus_channel_message_header *hdr) } /* +* Before setting channel->rescind in vmbus_rescind_cleanup(), we +* should make sure the channel callback is not running any more. +*/ + vmbus_reset_channel_cb(channel); + + /* * Now wait for offer handling to complete. */ vmbus_rescind_cleanup(channel); diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h index 3a3012f..5389012 100644 --- a/include/linux/hyperv.h +++ b/include/linux/hyperv.h @@ -1046,6 +1046,8 @@ extern int vmbus_establish_gpadl(struct vmbus_channel *channel, extern int vmbus_teardown_gpadl(struct vmbus_channel *channel, u32 gpadl_handle); +void vmbus_reset_channel_cb(struct vmbus_channel *channel); + extern int vmbus_recvpacket(struct vmbus_channel *channel, void *buffer, u32 bufferlen, -- 2.7.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 1/2] staging: mt7621-pci: use generic kernel pci subsystem read and write
On Fri, Jul 13, 2018 at 06:56:20AM +0200, Sergio Paracuellos wrote: > On Thu, Jul 12, 2018 at 09:28:15PM +0300, Dan Carpenter wrote: > > On Thu, Jul 12, 2018 at 08:15:00PM +0200, Sergio Paracuellos wrote: > > > map_bus callback is called before every .read/.write operation. > > > Implement it and change custom read write operations for the > > > pci subsystem generics. Make the probe function to assign data > > > for controller data and get pci register base from device tree. > > > > > > Signed-off-by: Sergio Paracuellos > > > --- > > > drivers/staging/mt7621-pci/pci-mt7621.c | 99 > > > +++-- > > > 1 file changed, 95 insertions(+), 4 deletions(-) > > > > > > diff --git a/drivers/staging/mt7621-pci/pci-mt7621.c > > > b/drivers/staging/mt7621-pci/pci-mt7621.c > > > index 4f56840..6187d41 100644 > > > --- a/drivers/staging/mt7621-pci/pci-mt7621.c > > > +++ b/drivers/staging/mt7621-pci/pci-mt7621.c > > > @@ -52,6 +52,9 @@ > > > #include > > > #include > > > #include > > > +#include > > > +#include > > > +#include > > > #include > > > > > > #include > > > @@ -177,6 +180,32 @@ static int pcie_link_status = 0; > > > #define PCI_ACCESS_WRITE_2 4 > > > #define PCI_ACCESS_WRITE_4 5 > > > > > > +/** > > > + * struct mt7621_pcie_port - PCIe port information > > > + * @base: IO mapped register base > > > + * @list: port list > > > + * @pcie: pointer to PCIe host info > > > + * @reset: pointer to port reset control > > > + */ > > > +struct mt7621_pcie_port { > > > + void __iomem *base; > > > + struct list_head list; > > > + struct mt7621_pcie *pcie; > > > + struct reset_control *reset; > > > +}; > > > + > > > +/** > > > + * struct mt7621_pcie - PCIe host information > > > + * @base: IO Mapped Register Base > > > + * @dev: Pointer to PCIe device > > > + * @ports: pointer to PCIe port information > > > + */ > > > +struct mt7621_pcie { > > > + void __iomem *base; > > > + struct device *dev; > > > + struct list_head ports; > > > +}; > > > + > > > static inline u32 mt7621_pci_get_cfgaddr(unsigned int bus, unsigned int > > > slot, > > >unsigned int func, unsigned int where) > > > { > > > @@ -296,15 +325,27 @@ pci_config_write(struct pci_bus *bus, unsigned int > > > devfn, int where, int size, u > > > } > > > } > > > > > > +static void __iomem *mt7621_pcie_map_bus(struct pci_bus *bus, > > > + unsigned int devfn, int where) > > > +{ > > > + struct mt7621_pcie *pcie = bus->sysdata; > > > + u32 address = mt7621_pci_get_cfgaddr(bus->number, PCI_SLOT(devfn), > > > + PCI_FUNC(devfn), where); > > > + > > > + writel(address, pcie->base + RALINK_PCI_CONFIG_ADDR); > > > + > > > + return pcie->base + RALINK_PCI_CONFIG_DATA_VIRTUAL_REG + (where & 3); > > > +} > > > + > > > struct pci_ops mt7621_pci_ops = { > > > - .read = pci_config_read, > > > - .write = pci_config_write, > > > + .map_bus= mt7621_pcie_map_bus, > > > + .read = pci_generic_config_read, > > > + .write = pci_generic_config_write, > > > }; > > > > > > static struct resource mt7621_res_pci_mem1; > > > static struct resource mt7621_res_pci_io1; > > > static struct pci_controller mt7621_controller = { > > > - .pci_ops= &mt7621_pci_ops, > > > .mem_resource = &mt7621_res_pci_mem1, > > > .io_resource= &mt7621_res_pci_io1, > > > }; > > > @@ -479,10 +520,61 @@ void setup_cm_memory_region(struct resource > > > *mem_resource) > > > } > > > } > > > > > > +static int mt7621_pcie_parse_dt(struct mt7621_pcie *pcie) > > > +{ > > > + struct device *dev = pcie->dev; > > > + struct device_node *node = dev->of_node; > > > + struct resource regs; > > > + const char *type; > > > + int err; > > > + > > > + type = of_get_property(node, "device_type", NULL); > > > + if (!type || strcmp(type, "pci") != 0) { > > > + dev_err(dev, "invalid \"device_type\" %s\n", type); > > > + return -EINVAL; > > > + } > > > + > > > + err = of_address_to_resource(node, 0, ®s); > > > + if (err) { > > > + dev_err(dev, "missing \"reg\" property\n"); > > > + return err; > > > + } > > > + > > > + pcie->base = devm_pci_remap_cfg_resource(dev, ®s); > > > + if (IS_ERR(pcie->base)) > > > + return PTR_ERR(pcie->base); > > > + > > > + return 0; > > > +} > > > + > > > static int mt7621_pci_probe(struct platform_device *pdev) > > > { > > > + struct device *dev = &pdev->dev; > > > + struct mt7621_pcie *pcie; > > > + struct pci_host_bridge *bridge; > > > + int err; > > > unsigned long val = 0; > > > > > > + bridge = devm_pci_alloc_host_bridge(dev, sizeof(*pcie)); > > > + if (!bridge) > > > + return -ENODEV; > > > + > > > + pcie = pci_host_bridge_priv(bridge); > > > + pcie->dev = dev; > > > + INIT_LIST_HEAD(&pcie->ports); > > > + > > > + err = mt7621_pcie_parse_dt(pcie); > > > + if (err) { > > > + dev_err(dev, "Parsing DT failed\n")
Re: [PATCH 1/2] staging: bcm2835-audio: Check if workqueue allocation failed
On Fri, Jul 13, 2018 at 12:54:16AM +0300, Tuomas Tynkkynen wrote: > @@ -424,7 +411,9 @@ int bcm2835_audio_open(struct bcm2835_alsa_stream > *alsa_stream) > int status; > int ret; > > - my_workqueue_init(alsa_stream); > + alsa_stream->my_wq = alloc_workqueue("my_queue", WQ_HIGHPRI, 1); > + if (!alsa_stream->my_wq) > + return -ENOMEM; > > ret = bcm2835_audio_open_connection(alsa_stream); > if (ret) { This patch is good but if bcm2835_audio_open_connection() fails then we need to release alsa_stream->my_wq. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/2] staging: bcm2835-audio: Check if workqueue allocation failed
On Fri, Jul 13, 2018 at 09:48:16AM +0300, Dan Carpenter wrote: > On Fri, Jul 13, 2018 at 12:54:16AM +0300, Tuomas Tynkkynen wrote: > > @@ -424,7 +411,9 @@ int bcm2835_audio_open(struct bcm2835_alsa_stream > > *alsa_stream) > > int status; > > int ret; > > > > - my_workqueue_init(alsa_stream); > > + alsa_stream->my_wq = alloc_workqueue("my_queue", WQ_HIGHPRI, 1); > > + if (!alsa_stream->my_wq) > > + return -ENOMEM; > > > > ret = bcm2835_audio_open_connection(alsa_stream); > > if (ret) { > > This patch is good but if bcm2835_audio_open_connection() fails then > we need to release alsa_stream->my_wq. Never mind, you handle it in the next patch. The bug *was* there in the original code as well, so that's a legit way to split the patches. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel