Re: [PATCH] staging: android: ion: refactory ion_alloc for kernel driver use
On Sat, Mar 30, 2019 at 02:32:35AM +, Zengtao (B) wrote: > >-Original Message- > >From: Greg Kroah-Hartman [mailto:gre...@linuxfoundation.org] > >Sent: Saturday, March 30, 2019 12:04 AM > >To: Zengtao (B) > >Cc: labb...@redhat.com; sumit.sem...@linaro.org; > >de...@driverdev.osuosl.org; Todd Kjos ; > >linux-ker...@vger.kernel.org; dri-de...@lists.freedesktop.org; > >linaro-mm-...@lists.linaro.org; Arve Hjønnevåg ; > >Joel Fernandes ; Martijn Coenen > >; Christian Brauner > >Subject: Re: [PATCH] staging: android: ion: refactory ion_alloc for kernel > >driver use > > > >On Sat, Mar 30, 2019 at 02:40:16AM +0800, Zeng Tao wrote: > >> There are two reasons for this patch: > >> 1. There are some potential requirements for ion_alloc in kernel > >> space, some media drivers need to allocate media buffers from ion > >> instead of buddy or dma framework, this is more convient and clean > >> very for media drivers. And In that case, ion is the only media buffer > >> provider, it's more easier to maintain. > > > >As this really is just DMA, what is wrong with the existing dma framework > >that makes it hard to use? You have seen all of the changes recently to it, > >right? > > The current dma framework is powerful enough(to me, and more complex ^_^) > , CMA, IOMMU are all integrated, it's good. But buffer sharing, statistics, > debug, > are not so friendly for media drivers(each driver has to do all, but > duplicate jobs). Then go add statistics and debugging to the dma code so that everyone benefits! thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 0/2] MT7621 PCIe PHY
This series adds support for the PCIe PHY found in the Mediatek MT7621 SoC. This is the first attempt to get feedback of what is missing in this driver to be promoted from staging. There is also a 'mt7621-pci' driver which is the controller part which is still in staging and is a client of this phy. Both drivers have been tested together in a gnubee1 board. Changes in v2: - Reorder patches to get bindings first in the series. - Don't use child nodes in the device tree. Use #phy-cells=1 instead. - Update driver code with new 'xlate' function for the new device tree. - Minor changes in driver's macros changing some spaces to tabs. Thanks in advance for your time. Best regards, Sergio Paracuellos Sergio Paracuellos (2): dt-bindings: phy: Add binding for Mediatek MT7621 PCIe PHY phy: ralink: Add PHY driver for MT7621 PCIe PHY .../bindings/phy/mediatek,mt7621-pci-phy.txt | 28 ++ drivers/phy/ralink/Kconfig| 7 + drivers/phy/ralink/Makefile | 1 + drivers/phy/ralink/phy-mt7621-pci.c | 401 ++ 4 files changed, 437 insertions(+) create mode 100644 Documentation/devicetree/bindings/phy/mediatek,mt7621-pci-phy.txt create mode 100644 drivers/phy/ralink/phy-mt7621-pci.c -- 2.19.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 2/2] phy: ralink: Add PHY driver for MT7621 PCIe PHY
This patch adds a driver for the PCIe PHY of MT7621 SoC. Signed-off-by: Sergio Paracuellos --- drivers/phy/ralink/Kconfig | 7 + drivers/phy/ralink/Makefile | 1 + drivers/phy/ralink/phy-mt7621-pci.c | 401 3 files changed, 409 insertions(+) create mode 100644 drivers/phy/ralink/phy-mt7621-pci.c diff --git a/drivers/phy/ralink/Kconfig b/drivers/phy/ralink/Kconfig index 14fd219535ef..87943a10f210 100644 --- a/drivers/phy/ralink/Kconfig +++ b/drivers/phy/ralink/Kconfig @@ -1,6 +1,13 @@ # # PHY drivers for Ralink platforms. # +config PHY_MT7621_PCI + tristate "MediaTek MT7621 PCI PHY Driver" + depends on RALINK && OF + select GENERIC_PHY + help + Say 'Y' here to add support for MediaTek MT7621 PCI PHY driver, + config PHY_RALINK_USB tristate "Ralink USB PHY driver" depends on RALINK || COMPILE_TEST diff --git a/drivers/phy/ralink/Makefile b/drivers/phy/ralink/Makefile index 5c9e326e8757..2052d5649863 100644 --- a/drivers/phy/ralink/Makefile +++ b/drivers/phy/ralink/Makefile @@ -1 +1,2 @@ +obj-$(CONFIG_PHY_MT7621_PCI) += phy-mt7621-pci.o obj-$(CONFIG_PHY_RALINK_USB) += phy-ralink-usb.o diff --git a/drivers/phy/ralink/phy-mt7621-pci.c b/drivers/phy/ralink/phy-mt7621-pci.c new file mode 100644 index ..118302c122ee --- /dev/null +++ b/drivers/phy/ralink/phy-mt7621-pci.c @@ -0,0 +1,401 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Mediatek MT7621 PCI PHY Driver + * Author: Sergio Paracuellos + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#define RALINK_CLKCFG1 0x30 +#define CHIP_REV_MT7621_E2 0x0101 + +#define PCIE_PORT_CLK_EN(x)BIT(24 + (x)) + +#define RG_PE1_PIPE_REG0x02c +#define RG_PE1_PIPE_RSTBIT(12) +#define RG_PE1_PIPE_CMD_FRCBIT(4) + +#define RG_P0_TO_P1_WIDTH 0x100 +#define RG_PE1_H_LCDDS_REG 0x49c +#define RG_PE1_H_LCDDS_PCW GENMASK(30, 0) +#define RG_PE1_H_LCDDS_PCW_VAL(x) ((0x7fff & (x)) << 0) + +#define RG_PE1_FRC_H_XTAL_REG 0x400 +#define RG_PE1_FRC_H_XTAL_TYPE BIT(8) +#define RG_PE1_H_XTAL_TYPE GENMASK(10, 9) +#define RG_PE1_H_XTAL_TYPE_VAL(x) ((0x3 & (x)) << 9) + +#define RG_PE1_FRC_PHY_REG 0x000 +#define RG_PE1_FRC_PHY_EN BIT(4) +#define RG_PE1_PHY_EN BIT(5) + +#define RG_PE1_H_PLL_REG 0x490 +#define RG_PE1_H_PLL_BCGENMASK(23, 22) +#define RG_PE1_H_PLL_BC_VAL(x) ((0x3 & (x)) << 22) +#define RG_PE1_H_PLL_BPGENMASK(21, 18) +#define RG_PE1_H_PLL_BP_VAL(x) ((0xf & (x)) << 18) +#define RG_PE1_H_PLL_IRGENMASK(15, 12) +#define RG_PE1_H_PLL_IR_VAL(x) ((0xf & (x)) << 12) +#define RG_PE1_H_PLL_ICGENMASK(11, 8) +#define RG_PE1_H_PLL_IC_VAL(x) ((0xf & (x)) << 8) +#define RG_PE1_H_PLL_PREDIVGENMASK(7, 6) +#define RG_PE1_H_PLL_PREDIV_VAL(x) ((0x3 & (x)) << 6) +#define RG_PE1_PLL_DIVEN GENMASK(3, 1) +#define RG_PE1_PLL_DIVEN_VAL(x)((0x7 & (x)) << 1) + +#define RG_PE1_H_PLL_FBKSEL_REG0x4bc +#define RG_PE1_H_PLL_FBKSELGENMASK(5, 4) +#define RG_PE1_H_PLL_FBKSEL_VAL(x) ((0x3 & (x)) << 4) + +#defineRG_PE1_H_LCDDS_SSC_PRD_REG 0x4a4 +#define RG_PE1_H_LCDDS_SSC_PRD GENMASK(15, 0) +#define RG_PE1_H_LCDDS_SSC_PRD_VAL(x) ((0x & (x)) << 0) + +#define RG_PE1_H_LCDDS_SSC_DELTA_REG 0x4a8 +#define RG_PE1_H_LCDDS_SSC_DELTA GENMASK(11, 0) +#define RG_PE1_H_LCDDS_SSC_DELTA_VAL(x)((0xfff & (x)) << 0) +#define RG_PE1_H_LCDDS_SSC_DELTA1 GENMASK(27, 16) +#define RG_PE1_H_LCDDS_SSC_DELTA1_VAL(x) ((0xff & (x)) << 16) + +#define RG_PE1_LCDDS_CLK_PH_INV_REG0x4a0 +#define RG_PE1_LCDDS_CLK_PH_INVBIT(5) + +#define RG_PE1_H_PLL_BR_REG0x4ac +#define RG_PE1_H_PLL_BRGENMASK(18, 16) +#define RG_PE1_H_PLL_BR_VAL(x) ((0x7 & (x)) << 16) + +#defineRG_PE1_MSTCKDIV_REG 0x414 +#define RG_PE1_MSTCKDIVGENMASK(7, 6) +#define RG_PE1_MSTCKDIV_VAL(x) ((0x3 & (x)) << 6) + +#define RG_PE1_FRC_MSTCKDIVBIT(5) + +#define MAX_PHYS 2 + +/** + * struct mt7621_pci_phy_instance - Mt7621 Pcie PHY device + * @phy: pointer to the kernel PHY device + * @port_base: base register + * @i
[PATCH v2 1/2] dt-bindings: phy: Add binding for Mediatek MT7621 PCIe PHY
Add bindings to describe Mediatek MT7621 PCIe PHY.
Signed-off-by: Sergio Paracuellos
---
.../bindings/phy/mediatek,mt7621-pci-phy.txt | 28 +++
1 file changed, 28 insertions(+)
create mode 100644
Documentation/devicetree/bindings/phy/mediatek,mt7621-pci-phy.txt
diff --git a/Documentation/devicetree/bindings/phy/mediatek,mt7621-pci-phy.txt
b/Documentation/devicetree/bindings/phy/mediatek,mt7621-pci-phy.txt
new file mode 100644
index ..a369d715378b
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/mediatek,mt7621-pci-phy.txt
@@ -0,0 +1,28 @@
+Mediatek Mt7621 PCIe PHY
+
+Required properties:
+- compatible: must be "mediatek,mt7621-pci-phy"
+- reg: base address and length of the PCIe PHY block
+- #phy-cells: must be <1> for pcie0_phy and for pcie1_phy.
+
+Example:
+ pcie0_phy: pcie-phy@1e149000 {
+ compatible = "mediatek,mt7621-pci-phy";
+ reg = <0x1e149000 0x0700>;
+ #phy-cells = <1>;
+ };
+
+ pcie1_phy: pcie-phy@1e14a000 {
+ compatible = "mediatek,mt7621-pci-phy";
+ reg = <0x1e14a000 0x0700>;
+ #phy-cells = <1>;
+ };
+
+ /* users of the PCIe phy */
+
+ pcie: pcie@1e14 {
+ ...
+ ...
+ phys = <&pcie0_phy 0>, <&pcie0_phy 1>, <&pcie1_phy 0>;
+ phy-names = "pcie-phy0", "pcie-phy1", "pcie-phy2";
+ };
--
2.19.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH] staging: android: ion: refactory ion_alloc for kernel driver use
>-Original Message- >From: Greg Kroah-Hartman [mailto:gre...@linuxfoundation.org] >Sent: Saturday, March 30, 2019 12:04 AM >To: Zengtao (B) >Cc: labb...@redhat.com; sumit.sem...@linaro.org; >de...@driverdev.osuosl.org; Todd Kjos ; >linux-ker...@vger.kernel.org; dri-de...@lists.freedesktop.org; >linaro-mm-...@lists.linaro.org; Arve Hjønnevåg ; >Joel Fernandes ; Martijn Coenen >; Christian Brauner >Subject: Re: [PATCH] staging: android: ion: refactory ion_alloc for kernel >driver use > >On Sat, Mar 30, 2019 at 02:40:16AM +0800, Zeng Tao wrote: >> There are two reasons for this patch: >> 1. There are some potential requirements for ion_alloc in kernel >> space, some media drivers need to allocate media buffers from ion >> instead of buddy or dma framework, this is more convient and clean >> very for media drivers. And In that case, ion is the only media buffer >> provider, it's more easier to maintain. > >As this really is just DMA, what is wrong with the existing dma framework >that makes it hard to use? You have seen all of the changes recently to it, >right? The current dma framework is powerful enough(to me, and more complex ^_^) , CMA, IOMMU are all integrated, it's good. But buffer sharing, statistics, debug, are not so friendly for media drivers(each driver has to do all, but duplicate jobs). > >thanks, > >greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH] staging: android: ion: refactory ion_alloc for kernel driver use
>-Original Message-
>From: Dan Carpenter [mailto:dan.carpen...@oracle.com]
>Sent: Friday, March 29, 2019 7:03 PM
>To: Zengtao (B)
>Cc: labb...@redhat.com; sumit.sem...@linaro.org;
>de...@driverdev.osuosl.org; Todd Kjos ; Greg
>Kroah-Hartman ;
>linux-ker...@vger.kernel.org; dri-de...@lists.freedesktop.org;
>linaro-mm-...@lists.linaro.org; Arve Hjønnevåg ;
>Joel Fernandes ; Martijn Coenen
>; Christian Brauner
>Subject: Re: [PATCH] staging: android: ion: refactory ion_alloc for kernel
>driver use
>
>If we're going to export ion_alloc() for other drivers to use then let's make
>an ion_free() helper function as well.
>
Good catch, thanks.
Regards
Zengtao
>void ion_free(struct dma_buf *dmabuf)
>{
> dma_buf_put(dmabuf);
>}
>
>regards,
>dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH] staging: android: ion: refactory ion_alloc for kernel driver use
>-Original Message-
>From: Greg Kroah-Hartman [mailto:gre...@linuxfoundation.org]
>Sent: Saturday, March 30, 2019 12:02 AM
>To: Zengtao (B)
>Cc: labb...@redhat.com; sumit.sem...@linaro.org;
>de...@driverdev.osuosl.org; Todd Kjos ;
>linux-ker...@vger.kernel.org; dri-de...@lists.freedesktop.org;
>linaro-mm-...@lists.linaro.org; Arve Hjønnevåg ;
>Joel Fernandes ; Martijn Coenen
>; Christian Brauner
>Subject: Re: [PATCH] staging: android: ion: refactory ion_alloc for kernel
>driver use
>
>On Sat, Mar 30, 2019 at 02:40:16AM +0800, Zeng Tao wrote:
>> There are two reasons for this patch:
>> 1. There are some potential requirements for ion_alloc in kernel
>> space, some media drivers need to allocate media buffers from ion
>> instead of buddy or dma framework, this is more convient and clean
>> very for media drivers. And In that case, ion is the only media buffer
>> provider, it's more easier to maintain.
>> 2. Fd is only needed by user processes, not the kernel space, so
>> dma_buf should be returned instead of fd for kernel space, and
>> dma_buf_fd should be called only for userspace api.
>>
>> Signed-off-by: Zeng Tao
>> ---
>> drivers/staging/android/ion/ion.c | 32
>> +---
>> 1 file changed, 17 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/staging/android/ion/ion.c
>> b/drivers/staging/android/ion/ion.c
>> index 92c2914..e93fb49 100644
>> --- a/drivers/staging/android/ion/ion.c
>> +++ b/drivers/staging/android/ion/ion.c
>> @@ -387,13 +387,13 @@ static const struct dma_buf_ops
>dma_buf_ops = {
>> .unmap = ion_dma_buf_kunmap,
>> };
>>
>> -static int ion_alloc(size_t len, unsigned int heap_id_mask, unsigned
>> int flags)
>> +struct dma_buf *ion_alloc(size_t len, unsigned int heap_id_mask,
>> + unsigned int flags)
>> {
>> struct ion_device *dev = internal_dev;
>> struct ion_buffer *buffer = NULL;
>> struct ion_heap *heap;
>> DEFINE_DMA_BUF_EXPORT_INFO(exp_info);
>> -int fd;
>> struct dma_buf *dmabuf;
>>
>> pr_debug("%s: len %zu heap_id_mask %u flags %x\n", __func__,
>@@
>> -407,7 +407,7 @@ static int ion_alloc(size_t len, unsigned int
>heap_id_mask, unsigned int flags)
>> len = PAGE_ALIGN(len);
>>
>> if (!len)
>> -return -EINVAL;
>> +return ERR_PTR(-EINVAL);
>>
>> down_read(&dev->lock);
>> plist_for_each_entry(heap, &dev->heaps, node) { @@ -421,10
>+421,10
>> @@ static int ion_alloc(size_t len, unsigned int heap_id_mask,
>unsigned int flags)
>> up_read(&dev->lock);
>>
>> if (!buffer)
>> -return -ENODEV;
>> +return ERR_PTR(-ENODEV);
>>
>> if (IS_ERR(buffer))
>> -return PTR_ERR(buffer);
>> +return ERR_PTR(PTR_ERR(buffer));
>>
>> exp_info.ops = &dma_buf_ops;
>> exp_info.size = buffer->size;
>> @@ -432,17 +432,12 @@ static int ion_alloc(size_t len, unsigned int
>heap_id_mask, unsigned int flags)
>> exp_info.priv = buffer;
>>
>> dmabuf = dma_buf_export(&exp_info);
>> -if (IS_ERR(dmabuf)) {
>> +if (IS_ERR(dmabuf))
>> _ion_buffer_destroy(buffer);
>> -return PTR_ERR(dmabuf);
>> -}
>>
>> -fd = dma_buf_fd(dmabuf, O_CLOEXEC);
>> -if (fd < 0)
>> -dma_buf_put(dmabuf);
>> -
>> -return fd;
>> +return dmabuf;
>> }
>> +EXPORT_SYMBOL(ion_alloc);
>
>If you are going to do this (and personally I'm with Laura in that I don't
>think you need it) this should be EXPORT_SYMBOL_GPL() please.
>
Agree, thanks
Regards
Zengtao
>thanks,
>
>greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH] staging: android: ion: refactory ion_alloc for kernel driver use
Hi laura:
>-Original Message-
>From: Laura Abbott [mailto:labb...@redhat.com]
>Sent: Friday, March 29, 2019 9:27 PM
>To: Zengtao (B) ; sumit.sem...@linaro.org
>Cc: Greg Kroah-Hartman ; Arve Hjønnevåg
>; Todd Kjos ; Martijn Coenen
>; Joel Fernandes ;
>Christian Brauner ; de...@driverdev.osuosl.org;
>dri-de...@lists.freedesktop.org; linaro-mm-...@lists.linaro.org;
>linux-ker...@vger.kernel.org
>Subject: Re: [PATCH] staging: android: ion: refactory ion_alloc for kernel
>driver use
>
>On 3/29/19 11:40 AM, Zeng Tao wrote:
>> There are two reasons for this patch:
>> 1. There are some potential requirements for ion_alloc in kernel
>> space, some media drivers need to allocate media buffers from ion
>> instead of buddy or dma framework, this is more convient and clean
>> very for media drivers. And In that case, ion is the only media buffer
>> provider, it's more easier to maintain.
>> 2. Fd is only needed by user processes, not the kernel space, so
>> dma_buf should be returned instead of fd for kernel space, and
>> dma_buf_fd should be called only for userspace api.
>>
>
>I really want to just NAK this because it doesn't seem like something
>that's necessary. The purpose of Ion is to provide buffers to userspace
>because there's no other way for userspace to get access to the memory.
>The kernel already has other APIs to access the memory. This also
>complicates the re-work that's been happening where the requirement is
>only userspace.
>
>Can you be more detailed about which media drivers you are referring to
>and why they can't just use other APIs?
>
I think I 've got your point, the ION is designed for usespace, but for kernel
space, we are really lacking of someone which plays the same role,(allocate
media memory, share the memory using dma_buf, provide debug and statistics
for media memory).
In fact, for kernel space, we have the dma framework, dma-buf, etc..
And we can work on top of such apis, but some duplicate jobs(everyone has
to maintain its own buffer sharing, debug and statistics).
So we need to have some to do the common things(ION's the best choice now)
When the ION was introduced, a lot of media memory frameworks existed, the
dma framework was not so good, so ION heaps, integrated buffer sharing,
statistics
and usespace api were the required features, but now dma framework is more
powerful,
we don't even need ION heaps now, but the userspace api, buffer sharing,
statistics are
still needed, and the buffer sharing, statistics can be re-worked and export to
kernel space,
not only used by userspace, , and that is my point.
>
>> Signed-off-by: Zeng Tao
>> ---
>> drivers/staging/android/ion/ion.c | 32
>+---
>> 1 file changed, 17 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/staging/android/ion/ion.c
>> b/drivers/staging/android/ion/ion.c
>> index 92c2914..e93fb49 100644
>> --- a/drivers/staging/android/ion/ion.c
>> +++ b/drivers/staging/android/ion/ion.c
>> @@ -387,13 +387,13 @@ static const struct dma_buf_ops
>dma_buf_ops = {
>> .unmap = ion_dma_buf_kunmap,
>> };
>>
>> -static int ion_alloc(size_t len, unsigned int heap_id_mask, unsigned
>> int flags)
>> +struct dma_buf *ion_alloc(size_t len, unsigned int heap_id_mask,
>> + unsigned int flags)
>> {
>> struct ion_device *dev = internal_dev;
>> struct ion_buffer *buffer = NULL;
>> struct ion_heap *heap;
>> DEFINE_DMA_BUF_EXPORT_INFO(exp_info);
>> -int fd;
>> struct dma_buf *dmabuf;
>>
>> pr_debug("%s: len %zu heap_id_mask %u flags %x\n", __func__,
>@@
>> -407,7 +407,7 @@ static int ion_alloc(size_t len, unsigned int
>heap_id_mask, unsigned int flags)
>> len = PAGE_ALIGN(len);
>>
>> if (!len)
>> -return -EINVAL;
>> +return ERR_PTR(-EINVAL);
>>
>> down_read(&dev->lock);
>> plist_for_each_entry(heap, &dev->heaps, node) { @@ -421,10
>+421,10
>> @@ static int ion_alloc(size_t len, unsigned int heap_id_mask,
>unsigned int flags)
>> up_read(&dev->lock);
>>
>> if (!buffer)
>> -return -ENODEV;
>> +return ERR_PTR(-ENODEV);
>>
>> if (IS_ERR(buffer))
>> -return PTR_ERR(buffer);
>> +return ERR_PTR(PTR_ERR(buffer));
>>
>> exp_info.ops = &dma_buf_ops;
>> exp_info.size = buffer->size;
>> @@ -432,17 +432,12 @@ static int ion_alloc(size_t len, unsigned int
>heap_id_mask, unsigned int flags)
>> exp_info.priv = buffer;
>>
>> dmabuf = dma_buf_export(&exp_info);
>> -if (IS_ERR(dmabuf)) {
>> +if (IS_ERR(dmabuf))
>> _ion_buffer_destroy(buffer);
>> -return PTR_ERR(dmabuf);
>> -}
>>
>> -fd = dma_buf_fd(dmabuf, O_CLOEXEC);
>> -if (fd < 0)
>> -dma_buf_put(dmabuf);
>> -
>> -return fd;
>> +return dmabuf;
>> }
>> +EXPORT_SYMBOL(ion_alloc);
>>
>> static int ion_query_heaps(struct ion_heap_query *query)
>> {
>> @@ -539,12 +534,19 @@ static long ion_ioctl(struct f
Re: [PATCH] staging: wlan-ng: Shorten excessively long p80211 constants
Hi Mikhail,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on staging/staging-testing]
[also build test WARNING on v5.1-rc2 next-20190329]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system]
url:
https://github.com/0day-ci/linux/commits/Mikhail-Gusarov/staging-wlan-ng-Shorten-excessively-long-p80211-constants/20190329-151335
reproduce:
# apt-get install sparse
make ARCH=x86_64 allmodconfig
make C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__'
sparse warnings: (new ones prefixed by >>)
drivers/staging/wlan-ng/p80211req.c:236:14: sparse: undefined identifier
'SMT_PRIVTABLE_EXCLUDEUNENCRYPTED'
>> drivers/staging/wlan-ng/p80211req.c:236:14: sparse: incompatible types for
>> 'case' statement
drivers/staging/wlan-ng/p80211req.c:236:14: sparse: Expected constant
expression in case statement
vim +/case +236 drivers/staging/wlan-ng/p80211req.c
197
198 static void p80211req_mibset_mibget(struct wlandevice *wlandev,
199 struct p80211msg_dot11req_mibget
*mib_msg,
200 int isget)
201 {
202 struct p80211itemd *mibitem =
203 (struct p80211itemd *)mib_msg->mibattribute.data;
204 struct p80211pstrd *pstr = (struct p80211pstrd *)mibitem->data;
205 u8 *key = mibitem->data + sizeof(struct p80211pstrd);
206
207 switch (mibitem->did) {
208 case smt_wepdefaultkeystable_key(1):
209 case smt_wepdefaultkeystable_key(2):
210 case smt_wepdefaultkeystable_key(3):
211 case smt_wepdefaultkeystable_key(4):
212 if (!isget)
213 wep_change_key(wlandev,
214 ITEM(0, mibitem->did) - 1,
215 key, pstr->len);
216 break;
217
218 case SMT_PRIVTABLE_WEPDEFAULTKEYID: {
219 u32 *data = (u32 *)mibitem->data;
220
221 if (isget) {
222 *data = wlandev->hostwep &
HOSTWEP_DEFAULTKEY_MASK;
223 } else {
224 wlandev->hostwep &= ~(HOSTWEP_DEFAULTKEY_MASK);
225 wlandev->hostwep |= (*data &
HOSTWEP_DEFAULTKEY_MASK);
226 }
227 break;
228 }
229 case SMT_PRIVTABLE_PRIVACYINVOKED: {
230 u32 *data = (u32 *)mibitem->data;
231
232 p80211req_handle_action(wlandev, data, isget,
233 HOSTWEP_PRIVACYINVOKED);
234 break;
235 }
> 236 case SMT_PRIVTABLE_EXCLUDEUNENCRYPTED: {
---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: rtl8723bs: core: fix line over 80 characters warning
On 3/26/2019 11:55 PM, Anirudh Rayabharam wrote:
Shorten the expression by re-using the part that was already computed to
fix the line over 80 characters warning reported by checkpatch.pl.
Signed-off-by: Anirudh Rayabharam
---
drivers/staging/rtl8723bs/core/rtw_ap.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/rtl8723bs/core/rtw_ap.c
b/drivers/staging/rtl8723bs/core/rtw_ap.c
index 18fabf5ff44b..bc0230672457 100644
--- a/drivers/staging/rtl8723bs/core/rtw_ap.c
+++ b/drivers/staging/rtl8723bs/core/rtw_ap.c
@@ -2336,8 +2336,8 @@ void rtw_ap_restore_network(struct adapter *padapter)
Update_RA_Entry(padapter, psta);
/* pairwise key */
/* per sta pairwise key and settings */
- if ((padapter->securitypriv.dot11PrivacyAlgrthm ==
_TKIP_) ||
- (padapter->securitypriv.dot11PrivacyAlgrthm ==
_AES_)) {
+ if ((psecuritypriv->dot11PrivacyAlgrthm == _TKIP_) ||
+ (psecuritypriv->dot11PrivacyAlgrthm == _AES_)) {
rtw_setstakey_cmd(padapter, psta, true, false);
}
Get rid of this {}.fix this .
Now patch looks good after Dan comment.
Reviewed-by: Mukesh Ojha
Cheers,
-Mukesh
}
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] media: staging: use strscpy() instead of strlcpy()
On 3/29/2019 8:59 PM, Mauro Carvalho Chehab wrote: There are a few left overs at staging with were still using the deprecated strlcpy() function. Signed-off-by: Mauro Carvalho Chehab s/with/which Reviewed-by: Mukesh Ojha Cheers, -Mukesh --- drivers/staging/media/imx/imx-media-dev-common.c | 4 ++-- drivers/staging/media/rockchip/vpu/rockchip_vpu_drv.c | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/staging/media/imx/imx-media-dev-common.c b/drivers/staging/media/imx/imx-media-dev-common.c index 910594125889..6cd93419b81d 100644 --- a/drivers/staging/media/imx/imx-media-dev-common.c +++ b/drivers/staging/media/imx/imx-media-dev-common.c @@ -30,7 +30,7 @@ struct imx_media_dev *imx_media_dev_init(struct device *dev) dev_set_drvdata(dev, imxmd); - strlcpy(imxmd->md.model, "imx-media", sizeof(imxmd->md.model)); + strscpy(imxmd->md.model, "imx-media", sizeof(imxmd->md.model)); imxmd->md.ops = &imx_media_md_ops; imxmd->md.dev = dev; @@ -38,7 +38,7 @@ struct imx_media_dev *imx_media_dev_init(struct device *dev) imxmd->v4l2_dev.mdev = &imxmd->md; imxmd->v4l2_dev.notify = imx_media_notify; - strlcpy(imxmd->v4l2_dev.name, "imx-media", + strscpy(imxmd->v4l2_dev.name, "imx-media", sizeof(imxmd->v4l2_dev.name)); media_device_init(&imxmd->md); diff --git a/drivers/staging/media/rockchip/vpu/rockchip_vpu_drv.c b/drivers/staging/media/rockchip/vpu/rockchip_vpu_drv.c index 57611464ad07..58721c46fba4 100644 --- a/drivers/staging/media/rockchip/vpu/rockchip_vpu_drv.c +++ b/drivers/staging/media/rockchip/vpu/rockchip_vpu_drv.c @@ -462,7 +462,7 @@ static int rockchip_vpu_probe(struct platform_device *pdev) } vpu->mdev.dev = vpu->dev; - strlcpy(vpu->mdev.model, DRIVER_NAME, sizeof(vpu->mdev.model)); + strscpy(vpu->mdev.model, DRIVER_NAME, sizeof(vpu->mdev.model)); media_device_init(&vpu->mdev); vpu->v4l2_dev.mdev = &vpu->mdev; ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2] staging: rtl8723bs: core: fix line over 80 characters warning
On Wed, Mar 27, 2019 at 11:49:07PM +0530, Anirudh Rayabharam wrote:
> Checkpatch.pl complains that these lines are over 80 characters. Use the
> "psecuritypriv" pointer for consistency, remove unnecessary parantheses
> and fix the alignment.
>
> This patch just cleans up a condition, it doesn't affect runtime.
>
> Signed-off-by: Anirudh Rayabharam
> Reviewed-by: Dan Carpenter
> ---
> v2: Made the commit message clearer, removed unnecessary parantheses and fixed
> the alignment as suggested by Dan Carpenter
>
> drivers/staging/rtl8723bs/core/rtw_ap.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/rtl8723bs/core/rtw_ap.c
> b/drivers/staging/rtl8723bs/core/rtw_ap.c
> index 18fabf5ff44b..8062b7f36de2 100644
> --- a/drivers/staging/rtl8723bs/core/rtw_ap.c
> +++ b/drivers/staging/rtl8723bs/core/rtw_ap.c
> @@ -2336,8 +2336,8 @@ void rtw_ap_restore_network(struct adapter *padapter)
> Update_RA_Entry(padapter, psta);
> /* pairwise key */
> /* per sta pairwise key and settings */
> - if ((padapter->securitypriv.dot11PrivacyAlgrthm ==
> _TKIP_) ||
> - (padapter->securitypriv.dot11PrivacyAlgrthm ==
> _AES_)) {
> + if (psecuritypriv->dot11PrivacyAlgrthm == _TKIP_ ||
> + psecuritypriv->dot11PrivacyAlgrthm == _AES_) {
> rtw_setstakey_cmd(padapter, psta, true, false);
> }
> }
Patch does not apply to my staging-next branch :(
Please rebase and resend.
thanks,
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: android: ion: refactory ion_alloc for kernel driver use
On Sat, Mar 30, 2019 at 02:40:16AM +0800, Zeng Tao wrote: > There are two reasons for this patch: > 1. There are some potential requirements for ion_alloc in kernel space, > some media drivers need to allocate media buffers from ion instead of > buddy or dma framework, this is more convient and clean very for media > drivers. And In that case, ion is the only media buffer provider, it's > more easier to maintain. As this really is just DMA, what is wrong with the existing dma framework that makes it hard to use? You have seen all of the changes recently to it, right? thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: android: ion: refactory ion_alloc for kernel driver use
On Sat, Mar 30, 2019 at 02:40:16AM +0800, Zeng Tao wrote:
> There are two reasons for this patch:
> 1. There are some potential requirements for ion_alloc in kernel space,
> some media drivers need to allocate media buffers from ion instead of
> buddy or dma framework, this is more convient and clean very for media
> drivers. And In that case, ion is the only media buffer provider, it's
> more easier to maintain.
> 2. Fd is only needed by user processes, not the kernel space, so dma_buf
> should be returned instead of fd for kernel space, and dma_buf_fd should
> be called only for userspace api.
>
> Signed-off-by: Zeng Tao
> ---
> drivers/staging/android/ion/ion.c | 32 +---
> 1 file changed, 17 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/staging/android/ion/ion.c
> b/drivers/staging/android/ion/ion.c
> index 92c2914..e93fb49 100644
> --- a/drivers/staging/android/ion/ion.c
> +++ b/drivers/staging/android/ion/ion.c
> @@ -387,13 +387,13 @@ static const struct dma_buf_ops dma_buf_ops = {
> .unmap = ion_dma_buf_kunmap,
> };
>
> -static int ion_alloc(size_t len, unsigned int heap_id_mask, unsigned int
> flags)
> +struct dma_buf *ion_alloc(size_t len, unsigned int heap_id_mask,
> + unsigned int flags)
> {
> struct ion_device *dev = internal_dev;
> struct ion_buffer *buffer = NULL;
> struct ion_heap *heap;
> DEFINE_DMA_BUF_EXPORT_INFO(exp_info);
> - int fd;
> struct dma_buf *dmabuf;
>
> pr_debug("%s: len %zu heap_id_mask %u flags %x\n", __func__,
> @@ -407,7 +407,7 @@ static int ion_alloc(size_t len, unsigned int
> heap_id_mask, unsigned int flags)
> len = PAGE_ALIGN(len);
>
> if (!len)
> - return -EINVAL;
> + return ERR_PTR(-EINVAL);
>
> down_read(&dev->lock);
> plist_for_each_entry(heap, &dev->heaps, node) {
> @@ -421,10 +421,10 @@ static int ion_alloc(size_t len, unsigned int
> heap_id_mask, unsigned int flags)
> up_read(&dev->lock);
>
> if (!buffer)
> - return -ENODEV;
> + return ERR_PTR(-ENODEV);
>
> if (IS_ERR(buffer))
> - return PTR_ERR(buffer);
> + return ERR_PTR(PTR_ERR(buffer));
>
> exp_info.ops = &dma_buf_ops;
> exp_info.size = buffer->size;
> @@ -432,17 +432,12 @@ static int ion_alloc(size_t len, unsigned int
> heap_id_mask, unsigned int flags)
> exp_info.priv = buffer;
>
> dmabuf = dma_buf_export(&exp_info);
> - if (IS_ERR(dmabuf)) {
> + if (IS_ERR(dmabuf))
> _ion_buffer_destroy(buffer);
> - return PTR_ERR(dmabuf);
> - }
>
> - fd = dma_buf_fd(dmabuf, O_CLOEXEC);
> - if (fd < 0)
> - dma_buf_put(dmabuf);
> -
> - return fd;
> + return dmabuf;
> }
> +EXPORT_SYMBOL(ion_alloc);
If you are going to do this (and personally I'm with Laura in that I
don't think you need it) this should be EXPORT_SYMBOL_GPL() please.
thanks,
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] media: staging: use strscpy() instead of strlcpy()
On 3/29/19 4:29 PM, Mauro Carvalho Chehab wrote: > There are a few left overs at staging with were still using the > deprecated strlcpy() function. > > Signed-off-by: Mauro Carvalho Chehab Reviewed-by: Hans Verkuil Thanks! Hans > --- > drivers/staging/media/imx/imx-media-dev-common.c | 4 ++-- > drivers/staging/media/rockchip/vpu/rockchip_vpu_drv.c | 2 +- > 2 files changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/staging/media/imx/imx-media-dev-common.c > b/drivers/staging/media/imx/imx-media-dev-common.c > index 910594125889..6cd93419b81d 100644 > --- a/drivers/staging/media/imx/imx-media-dev-common.c > +++ b/drivers/staging/media/imx/imx-media-dev-common.c > @@ -30,7 +30,7 @@ struct imx_media_dev *imx_media_dev_init(struct device *dev) > > dev_set_drvdata(dev, imxmd); > > - strlcpy(imxmd->md.model, "imx-media", sizeof(imxmd->md.model)); > + strscpy(imxmd->md.model, "imx-media", sizeof(imxmd->md.model)); > imxmd->md.ops = &imx_media_md_ops; > imxmd->md.dev = dev; > > @@ -38,7 +38,7 @@ struct imx_media_dev *imx_media_dev_init(struct device *dev) > > imxmd->v4l2_dev.mdev = &imxmd->md; > imxmd->v4l2_dev.notify = imx_media_notify; > - strlcpy(imxmd->v4l2_dev.name, "imx-media", > + strscpy(imxmd->v4l2_dev.name, "imx-media", > sizeof(imxmd->v4l2_dev.name)); > > media_device_init(&imxmd->md); > diff --git a/drivers/staging/media/rockchip/vpu/rockchip_vpu_drv.c > b/drivers/staging/media/rockchip/vpu/rockchip_vpu_drv.c > index 57611464ad07..58721c46fba4 100644 > --- a/drivers/staging/media/rockchip/vpu/rockchip_vpu_drv.c > +++ b/drivers/staging/media/rockchip/vpu/rockchip_vpu_drv.c > @@ -462,7 +462,7 @@ static int rockchip_vpu_probe(struct platform_device > *pdev) > } > > vpu->mdev.dev = vpu->dev; > - strlcpy(vpu->mdev.model, DRIVER_NAME, sizeof(vpu->mdev.model)); > + strscpy(vpu->mdev.model, DRIVER_NAME, sizeof(vpu->mdev.model)); > media_device_init(&vpu->mdev); > vpu->v4l2_dev.mdev = &vpu->mdev; > > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] media: staging: use strscpy() instead of strlcpy()
There are a few left overs at staging with were still using the deprecated strlcpy() function. Signed-off-by: Mauro Carvalho Chehab --- drivers/staging/media/imx/imx-media-dev-common.c | 4 ++-- drivers/staging/media/rockchip/vpu/rockchip_vpu_drv.c | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/staging/media/imx/imx-media-dev-common.c b/drivers/staging/media/imx/imx-media-dev-common.c index 910594125889..6cd93419b81d 100644 --- a/drivers/staging/media/imx/imx-media-dev-common.c +++ b/drivers/staging/media/imx/imx-media-dev-common.c @@ -30,7 +30,7 @@ struct imx_media_dev *imx_media_dev_init(struct device *dev) dev_set_drvdata(dev, imxmd); - strlcpy(imxmd->md.model, "imx-media", sizeof(imxmd->md.model)); + strscpy(imxmd->md.model, "imx-media", sizeof(imxmd->md.model)); imxmd->md.ops = &imx_media_md_ops; imxmd->md.dev = dev; @@ -38,7 +38,7 @@ struct imx_media_dev *imx_media_dev_init(struct device *dev) imxmd->v4l2_dev.mdev = &imxmd->md; imxmd->v4l2_dev.notify = imx_media_notify; - strlcpy(imxmd->v4l2_dev.name, "imx-media", + strscpy(imxmd->v4l2_dev.name, "imx-media", sizeof(imxmd->v4l2_dev.name)); media_device_init(&imxmd->md); diff --git a/drivers/staging/media/rockchip/vpu/rockchip_vpu_drv.c b/drivers/staging/media/rockchip/vpu/rockchip_vpu_drv.c index 57611464ad07..58721c46fba4 100644 --- a/drivers/staging/media/rockchip/vpu/rockchip_vpu_drv.c +++ b/drivers/staging/media/rockchip/vpu/rockchip_vpu_drv.c @@ -462,7 +462,7 @@ static int rockchip_vpu_probe(struct platform_device *pdev) } vpu->mdev.dev = vpu->dev; - strlcpy(vpu->mdev.model, DRIVER_NAME, sizeof(vpu->mdev.model)); + strscpy(vpu->mdev.model, DRIVER_NAME, sizeof(vpu->mdev.model)); media_device_init(&vpu->mdev); vpu->v4l2_dev.mdev = &vpu->mdev; -- 2.20.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 04/14] staging: most: sound: introduce new sound adapter management
On Fr, 2019-03-29 at 16:50 +0300, Dan Carpenter wrote: > External E-Mail > > > Thanks, I feel like I understand better now. > > Sorry, I don't want to be a jerk and I'm not going to complain again > about this patchset if you resend it with the other stuff I mention > fixed. > > But to me it feels like the API could be improved slightly if you > first created the adapter with a name and then linked to > it. Creating > the adapter as a side effect of linking the first audio component > feels > like it seems helpful but it's going to be awkward. Hmm, I see. But I'm not sure if I can call snd_pcm_new() on an already registered sound adapter. > > I'm also concerned about the locking around the adpt_list. Is this > something ordinary users can trigger? Is it crash if I set my fuzz > tester to start randomly connecting and disconnecting like crazy? > Well, strictly speaking yes. Because it is user space. But normally, the driver is meant for engineered systems like a car (see Automotive Grade Linux). Here you will have a systemd unit that sets things up at system startup time. I don't expect having to deal with multiple user space applications that race about the sound adapters. But I will keep that in mind. thanks, Chris > regards, > dan carpenter > > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 04/14] staging: most: sound: introduce new sound adapter management
Thanks, I feel like I understand better now. Sorry, I don't want to be a jerk and I'm not going to complain again about this patchset if you resend it with the other stuff I mention fixed. But to me it feels like the API could be improved slightly if you first created the adapter with a name and then linked to it. Creating the adapter as a side effect of linking the first audio component feels like it seems helpful but it's going to be awkward. I'm also concerned about the locking around the adpt_list. Is this something ordinary users can trigger? Is it crash if I set my fuzz tester to start randomly connecting and disconnecting like crazy? regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: android: ion: refactory ion_alloc for kernel driver use
On 3/29/19 11:40 AM, Zeng Tao wrote:
There are two reasons for this patch:
1. There are some potential requirements for ion_alloc in kernel space,
some media drivers need to allocate media buffers from ion instead of
buddy or dma framework, this is more convient and clean very for media
drivers. And In that case, ion is the only media buffer provider, it's
more easier to maintain.
2. Fd is only needed by user processes, not the kernel space, so dma_buf
should be returned instead of fd for kernel space, and dma_buf_fd should
be called only for userspace api.
I really want to just NAK this because it doesn't seem like something
that's necessary. The purpose of Ion is to provide buffers to userspace
because there's no other way for userspace to get access to the memory.
The kernel already has other APIs to access the memory. This also
complicates the re-work that's been happening where the requirement
is only userspace.
Can you be more detailed about which media drivers you are referring
to and why they can't just use other APIs?
Signed-off-by: Zeng Tao
---
drivers/staging/android/ion/ion.c | 32 +---
1 file changed, 17 insertions(+), 15 deletions(-)
diff --git a/drivers/staging/android/ion/ion.c
b/drivers/staging/android/ion/ion.c
index 92c2914..e93fb49 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -387,13 +387,13 @@ static const struct dma_buf_ops dma_buf_ops = {
.unmap = ion_dma_buf_kunmap,
};
-static int ion_alloc(size_t len, unsigned int heap_id_mask, unsigned int flags)
+struct dma_buf *ion_alloc(size_t len, unsigned int heap_id_mask,
+ unsigned int flags)
{
struct ion_device *dev = internal_dev;
struct ion_buffer *buffer = NULL;
struct ion_heap *heap;
DEFINE_DMA_BUF_EXPORT_INFO(exp_info);
- int fd;
struct dma_buf *dmabuf;
pr_debug("%s: len %zu heap_id_mask %u flags %x\n", __func__,
@@ -407,7 +407,7 @@ static int ion_alloc(size_t len, unsigned int heap_id_mask,
unsigned int flags)
len = PAGE_ALIGN(len);
if (!len)
- return -EINVAL;
+ return ERR_PTR(-EINVAL);
down_read(&dev->lock);
plist_for_each_entry(heap, &dev->heaps, node) {
@@ -421,10 +421,10 @@ static int ion_alloc(size_t len, unsigned int
heap_id_mask, unsigned int flags)
up_read(&dev->lock);
if (!buffer)
- return -ENODEV;
+ return ERR_PTR(-ENODEV);
if (IS_ERR(buffer))
- return PTR_ERR(buffer);
+ return ERR_PTR(PTR_ERR(buffer));
exp_info.ops = &dma_buf_ops;
exp_info.size = buffer->size;
@@ -432,17 +432,12 @@ static int ion_alloc(size_t len, unsigned int
heap_id_mask, unsigned int flags)
exp_info.priv = buffer;
dmabuf = dma_buf_export(&exp_info);
- if (IS_ERR(dmabuf)) {
+ if (IS_ERR(dmabuf))
_ion_buffer_destroy(buffer);
- return PTR_ERR(dmabuf);
- }
- fd = dma_buf_fd(dmabuf, O_CLOEXEC);
- if (fd < 0)
- dma_buf_put(dmabuf);
-
- return fd;
+ return dmabuf;
}
+EXPORT_SYMBOL(ion_alloc);
static int ion_query_heaps(struct ion_heap_query *query)
{
@@ -539,12 +534,19 @@ static long ion_ioctl(struct file *filp, unsigned int
cmd, unsigned long arg)
case ION_IOC_ALLOC:
{
int fd;
+ struct dma_buf *dmabuf;
- fd = ion_alloc(data.allocation.len,
+ dmabuf = ion_alloc(data.allocation.len,
data.allocation.heap_id_mask,
data.allocation.flags);
- if (fd < 0)
+ if (IS_ERR(dmabuf))
+ return PTR_ERR(dmabuf);
+
+ fd = dma_buf_fd(dmabuf, O_CLOEXEC);
+ if (fd < 0) {
+ dma_buf_put(dmabuf);
return fd;
+ }
data.allocation.fd = fd;
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 04/14] staging: most: sound: introduce new sound adapter management
On Fr, 2019-03-29 at 13:46 +0300, Dan Carpenter wrote:
> External E-Mail
>
>
> On Thu, Mar 28, 2019 at 02:17:32PM +0100, Christian Gromm wrote:
> >
> > +static int audio_create_sound_card(void)
> > +{
> > + int ret;
> > + struct sound_adapter *adpt;
> > +
> > + list_for_each_entry(adpt, &adpt_list, list) {
> > + if (!adpt->registered)
> > + goto adpt_alloc;
> > + }
> > + return -ENODEV;
> > +adpt_alloc:
> > + ret = snd_card_register(adpt->card);
> > + if (ret < 0) {
> > + release_adapter(adpt);
> > + return ret;
> > + }
> > + adpt->registered = true;
> > + return ret;
> ^^
>
> return 0;
>
> >
> > +}
> This function is just strange to me... We add a bunch of adapters to
> "adpt_list" in audio_probe_channel(). Each adapter has it's own
> adpt->card. But here we just take the first unregistered adapter and
> register it as our card.
>
Unfortunately, I am not an ASCII artist, otherwise I would have put
some flowcharts in here to make things clear.
But in principle, it is like this:
A channel of a device which is intended to carry audio data,
will be represented as a PCM device on a sound card. So, if
I want my sound card
--> mkdir /sys/kernel/config/most_sound/mycard
to have a capture device and a playback device, I would
link two channels (one rx and on tx) and hook them on the card by
creating a directory in the card's directory.
--> mkdir /sys/kernel/config/most_sound/mycard/pcm_capture
--> mkdir
/sys/kernel/config/most_sound/mycard/pcm_playback
Then I would go ahead and configure the attributes inside and
finally activate the links:
--> echo 1 > /sys/kernel/config/most_sound/mycard/pcm_playback/create_link
and
--> echo 1 > /sys/kernel/config/most_sound/mycard/pcm_capture/create_link
The first time a link is being activated, the probe function gets
called and if no adapter has yet been created, one is created now
and the PCM device is added. By the time the second call happens,
we already have an adapter that is _not_ registered with ALSA.
Hence, we skip allocating another adapter and use the one we got
to add the next PCM device.
Once I have all PCM devices set up, I would do
--> echo 1 > /sys/kernel/config/most_sound/mycard/create_card
And now, the "strange" audio_create_sound_card function is
called to register the adapter with ALSA. If everything is fine,
the adpt->registered flag is set and from now on I would be able
to create a second sound card on the system. An only now a new
adapter would be allocated in the probe function, because we
can't find one in our list that is waiting to get registered
with ALSA.
Unless I do the create_card thing, all channels would be added
to the same adapter and I would end up with a sound card that
has "a bunch" of PCM devices.
Hope I kind of could get the picture across.
thanks,
Chris
> regards,
> dan carpenter
> ___
> devel mailing list
> de...@linuxdriverproject.org
> http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-de
> vel
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: android: ion: refactory ion_alloc for kernel driver use
If we're going to export ion_alloc() for other drivers to use then let's
make an ion_free() helper function as well.
void ion_free(struct dma_buf *dmabuf)
{
dma_buf_put(dmabuf);
}
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 04/14] staging: most: sound: introduce new sound adapter management
On Thu, Mar 28, 2019 at 02:17:32PM +0100, Christian Gromm wrote:
> +static int audio_create_sound_card(void)
> +{
> + int ret;
> + struct sound_adapter *adpt;
> +
> + list_for_each_entry(adpt, &adpt_list, list) {
> + if (!adpt->registered)
> + goto adpt_alloc;
> + }
> + return -ENODEV;
> +adpt_alloc:
> + ret = snd_card_register(adpt->card);
> + if (ret < 0) {
> + release_adapter(adpt);
> + return ret;
> + }
> + adpt->registered = true;
> + return ret;
^^
return 0;
> +}
This function is just strange to me... We add a bunch of adapters to
"adpt_list" in audio_probe_channel(). Each adapter has it's own
adpt->card. But here we just take the first unregistered adapter and
register it as our card.
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: android: ion: refactory ion_alloc for kernel driver use
There are two reasons for this patch:
1. There are some potential requirements for ion_alloc in kernel space,
some media drivers need to allocate media buffers from ion instead of
buddy or dma framework, this is more convient and clean very for media
drivers. And In that case, ion is the only media buffer provider, it's
more easier to maintain.
2. Fd is only needed by user processes, not the kernel space, so dma_buf
should be returned instead of fd for kernel space, and dma_buf_fd should
be called only for userspace api.
Signed-off-by: Zeng Tao
---
drivers/staging/android/ion/ion.c | 32 +---
1 file changed, 17 insertions(+), 15 deletions(-)
diff --git a/drivers/staging/android/ion/ion.c
b/drivers/staging/android/ion/ion.c
index 92c2914..e93fb49 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -387,13 +387,13 @@ static const struct dma_buf_ops dma_buf_ops = {
.unmap = ion_dma_buf_kunmap,
};
-static int ion_alloc(size_t len, unsigned int heap_id_mask, unsigned int flags)
+struct dma_buf *ion_alloc(size_t len, unsigned int heap_id_mask,
+ unsigned int flags)
{
struct ion_device *dev = internal_dev;
struct ion_buffer *buffer = NULL;
struct ion_heap *heap;
DEFINE_DMA_BUF_EXPORT_INFO(exp_info);
- int fd;
struct dma_buf *dmabuf;
pr_debug("%s: len %zu heap_id_mask %u flags %x\n", __func__,
@@ -407,7 +407,7 @@ static int ion_alloc(size_t len, unsigned int heap_id_mask,
unsigned int flags)
len = PAGE_ALIGN(len);
if (!len)
- return -EINVAL;
+ return ERR_PTR(-EINVAL);
down_read(&dev->lock);
plist_for_each_entry(heap, &dev->heaps, node) {
@@ -421,10 +421,10 @@ static int ion_alloc(size_t len, unsigned int
heap_id_mask, unsigned int flags)
up_read(&dev->lock);
if (!buffer)
- return -ENODEV;
+ return ERR_PTR(-ENODEV);
if (IS_ERR(buffer))
- return PTR_ERR(buffer);
+ return ERR_PTR(PTR_ERR(buffer));
exp_info.ops = &dma_buf_ops;
exp_info.size = buffer->size;
@@ -432,17 +432,12 @@ static int ion_alloc(size_t len, unsigned int
heap_id_mask, unsigned int flags)
exp_info.priv = buffer;
dmabuf = dma_buf_export(&exp_info);
- if (IS_ERR(dmabuf)) {
+ if (IS_ERR(dmabuf))
_ion_buffer_destroy(buffer);
- return PTR_ERR(dmabuf);
- }
- fd = dma_buf_fd(dmabuf, O_CLOEXEC);
- if (fd < 0)
- dma_buf_put(dmabuf);
-
- return fd;
+ return dmabuf;
}
+EXPORT_SYMBOL(ion_alloc);
static int ion_query_heaps(struct ion_heap_query *query)
{
@@ -539,12 +534,19 @@ static long ion_ioctl(struct file *filp, unsigned int
cmd, unsigned long arg)
case ION_IOC_ALLOC:
{
int fd;
+ struct dma_buf *dmabuf;
- fd = ion_alloc(data.allocation.len,
+ dmabuf = ion_alloc(data.allocation.len,
data.allocation.heap_id_mask,
data.allocation.flags);
- if (fd < 0)
+ if (IS_ERR(dmabuf))
+ return PTR_ERR(dmabuf);
+
+ fd = dma_buf_fd(dmabuf, O_CLOEXEC);
+ if (fd < 0) {
+ dma_buf_put(dmabuf);
return fd;
+ }
data.allocation.fd = fd;
--
2.7.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: kernel BUG at drivers/android/binder_alloc.c:LINE! (3)
Hello, syzbot has tested the proposed patch and the reproducer did not trigger crash: Reported-and-tested-by: syzbot+f9f3f388440283da2...@syzkaller.appspotmail.com Tested on: commit: 8c2ffd91 Linux 5.1-rc2 git tree: git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git master kernel config: https://syzkaller.appspot.com/x/.config?x=8dcdce25ea72bedf compiler: gcc (GCC) 9.0.0 20181231 (experimental) patch: https://syzkaller.appspot.com/x/patch.diff?x=10fed66320 Note: testing is done by a robot and is best-effort only. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: wlan-ng: Shorten excessively long p80211 constants
Hi Mikhail,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on staging/staging-testing]
[also build test ERROR on v5.1-rc2 next-20190329]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system]
url:
https://github.com/0day-ci/linux/commits/Mikhail-Gusarov/staging-wlan-ng-Shorten-excessively-long-p80211-constants/20190329-151335
config: xtensa-allyesconfig (attached as .config)
compiler: xtensa-linux-gcc (GCC) 8.1.0
reproduce:
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
GCC_VERSION=8.1.0 make.cross ARCH=xtensa
All errors (new ones prefixed by >>):
In file included from drivers/staging/wlan-ng/prism2usb.c:4:
>> drivers/staging/wlan-ng/prism2mib.c:176:3: error:
>> 'SMT_PRIVTABLE_EXCLUDEUNENCRYPTED' undeclared here (not in a function); did
>> you mean 'SMT_PRIVTABLE_EXUNENCRYPTED'?
{SMT_PRIVTABLE_EXCLUDEUNENCRYPTED,
^~~~
SMT_PRIVTABLE_EXUNENCRYPTED
--
drivers/staging/wlan-ng/p80211req.c: In function 'p80211req_mibset_mibget':
>> drivers/staging/wlan-ng/p80211req.c:236:7: error:
>> 'SMT_PRIVTABLE_EXCLUDEUNENCRYPTED' undeclared (first use in this function);
>> did you mean 'SMT_PRIVTABLE_EXUNENCRYPTED'?
case SMT_PRIVTABLE_EXCLUDEUNENCRYPTED: {
^~~~
SMT_PRIVTABLE_EXUNENCRYPTED
drivers/staging/wlan-ng/p80211req.c:236:7: note: each undeclared identifier
is reported only once for each function it appears in
vim +176 drivers/staging/wlan-ng/prism2mib.c
95
96 static int prism2mib_bytearea2pstr(struct mibrec *mib,
97 int isget,
98 struct wlandevice *wlandev,
99 struct hfa384x *hw,
100 struct p80211msg_dot11req_mibset
*msg,
101 void *data);
102
103 static int prism2mib_uint32(struct mibrec *mib,
104 int isget,
105 struct wlandevice *wlandev,
106 struct hfa384x *hw,
107 struct p80211msg_dot11req_mibset *msg, void
*data);
108
109 static int prism2mib_flag(struct mibrec *mib,
110int isget,
111struct wlandevice *wlandev,
112struct hfa384x *hw,
113struct p80211msg_dot11req_mibset *msg, void
*data);
114
115 static int prism2mib_wepdefaultkey(struct mibrec *mib,
116 int isget,
117 struct wlandevice *wlandev,
118 struct hfa384x *hw,
119 struct p80211msg_dot11req_mibset
*msg,
120 void *data);
121
122 static int prism2mib_privacyinvoked(struct mibrec *mib,
123 int isget,
124 struct wlandevice *wlandev,
125 struct hfa384x *hw,
126 struct p80211msg_dot11req_mibset
*msg,
127 void *data);
128
129 static int prism2mib_excludeunencrypted(struct mibrec *mib,
130 int isget,
131 struct wlandevice *wlandev,
132 struct hfa384x *hw,
133 struct
p80211msg_dot11req_mibset *msg,
134 void *data);
135
136 static int
137 prism2mib_fragmentationthreshold(struct mibrec *mib,
138 int isget,
139 struct wlandevice *wlandev,
140 struct hfa384x *hw,
141 struct p80211msg_dot11req_mibset *msg,
142 void *data);
143
144 static int prism2mib_priv(struct mibrec *mib,
145int isget,
146struct wlandevice *wlandev,
147struct hfa384x *hw,
148struct p80211msg_dot11req_mibset *msg, void
*data);
149
150 static struct mibrec mibtab[] = {
151 /* dot11smt MIB's */
152 {smt_wepdefaultkeystable_key(1),
153 F_STA | F_WRITE,
154
Re: [PATCH v2 04/14] staging: most: sound: introduce new sound adapter management
On Do, 2019-03-28 at 16:56 +0300, Dan Carpenter wrote:
> External E-Mail
>
>
> On Thu, Mar 28, 2019 at 02:17:32PM +0100, Christian Gromm wrote:
> >
> > +static int audio_create_sound_card(void)
> > +{
> > + int ret;
> > + struct sound_adapter *adpt;
> > +
> > + list_for_each_entry(adpt, &adpt_list, list) {
> > + if (!adpt->registered)
> > + goto adpt_alloc;
> > + }
> > + return -ENODEV;
> > +adpt_alloc:
> > + ret = snd_card_register(adpt->card);
> > + if (ret < 0) {
> > + release_adapter(adpt);
> This doesn't feel right. We didn't acquire "adpt" in this function
> so
> why are we releasing it here. Do we release it somewhere else as
> well?
>
We release the adapter, because it is useless if we have not been
able to register it with ALSA.
And yes, it is also being removed, when a channel gets disconnected.
> It's still on the list...
It is being removed from the list inside the function
release_adapter.
thanks,
Chris
>
> >
> > + return ret;
> > + }
> > + adpt->registered = true;
> > + return ret;
> > +}
> regards,
> dan carpenter
>
>
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 10/14] staging: most: allow speculative configuration
On Fri, Mar 29, 2019 at 09:15:46AM +, christian.gr...@microchip.com wrote:
> > > +static int set_config_and_add_link(struct mdev_link *mdev_link)
> > > +{
> > > + int i;
> > > + int ret;
> > > +
> > > + for (i = 0; i < ARRAY_SIZE(set_config_val); i++) {
> > > + ret = set_config_val[i](mdev_link);
> > > + if (ret == -ENODATA) {
> > I've read the commit description but I don't really understand the
> > error codes. Here only -ENODATA is an error. But later -ENODEV
> > is success. Why not "if (ret) {" here?
> >
>
> The most_set_cfg_* functions return success, ENODEV or ENODATA.
> We want to stop only in case there is something wrong with the
> provided data. A missing device is not an issue here. In this
> case we want to keep the configuration as is and complete stuff
> once a device is being attached.
>
Yeah... But future programmers will just add whatever error codes
occur to them at the time. Now we have two error codes which are
special instead of one (twice as much chance for them to mess up).
Just do it like:
if (ret < 0 && ret != -ENODEV) {
That's a more standard way to handle this.
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 01/14] staging: most: add new file configfs.c
On Do, 2019-03-28 at 16:50 +0300, Dan Carpenter wrote:
> External E-Mail
>
>
> On Thu, Mar 28, 2019 at 02:17:29PM +0100, Christian Gromm wrote:
> >
> > +static ssize_t mdev_link_direction_store(struct config_item *item,
> > + const char *page, size_t
> > count)
> > +{
> > + struct mdev_link *mdev_link = to_mdev_link(item);
> > +
> > + if (sysfs_streq(page, "dir_rx") && sysfs_streq(page, "rx")
> > &&
> > + sysfs_streq(page, "dir_tx") && sysfs_streq(page,
> > "tx"))
> These tests are reversed. It will never return -EINVAL because one
> string can't be four things.
OMG, that was braindead. Sorry for the inconvenience. I'll fix that up.
thanks,
Chris
>
> if (!sysfs_streq(page, "dir_rx") && !sysfs_streq(page, "rx") &&
> !sysfs_streq(page, "dir_tx") && !sysfs_streq(page, "tx"))
> return -EINVAL;
>
> The sysfs_streq() return true if the strings are equal. The strcmp()
> functions less intuitive and they should be used like this:
>
> if (strcmp(foo, bar) < 0) { // <-- foo < bar
> if (strcmp(foo, bar) != 0) { // <-- foo != bar
> if (strcmp(foo, bar) == 0) { // <-- foo == bar
>
> The other streq() tests have the same issue.
>
> regards,
> dan carpenter
>
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 10/14] staging: most: allow speculative configuration
On Do, 2019-03-28 at 17:12 +0300, Dan Carpenter wrote:
> External E-Mail
>
>
> On Thu, Mar 28, 2019 at 02:17:38PM +0100, Christian Gromm wrote:
> >
> > This patch makes the driver accept a link confiiguration eventhough
> > no
> > device is attached to the bus. Instead the configuration is being
> > applied
> > as soon as a device is being registered with the core.
> >
> > Signed-off-by: Christian Gromm
> > ---
> > v2:
> > - follow-up adaptions due to changes introduced w/ [PATCH v2
> > 01/14]
> >
> > drivers/staging/most/configfs.c| 60
> > --
> > drivers/staging/most/core.c| 16 +++---
> > drivers/staging/most/core.h| 1 +
> > drivers/staging/most/sound/sound.c | 6 ++--
> > 4 files changed, 53 insertions(+), 30 deletions(-)
> >
> > diff --git a/drivers/staging/most/configfs.c
> > b/drivers/staging/most/configfs.c
> > index 6968b299..3a7c54d 100644
> > --- a/drivers/staging/most/configfs.c
> > +++ b/drivers/staging/most/configfs.c
> > @@ -14,6 +14,7 @@
> >
> > struct mdev_link {
> > struct config_item item;
> > + struct list_head list;
> > bool create;
> > u16 num_buffers;
> > u16 buffer_size;
> > @@ -29,6 +30,8 @@ struct mdev_link {
> > char param[PAGE_SIZE];
> > };
> >
> > +struct list_head mdev_link_list;
> > +
> > static int set_cfg_buffer_size(struct mdev_link *link)
> > {
> > if (!link->buffer_size)
> > @@ -105,33 +108,41 @@ static ssize_t mdev_link_create_show(struct
> > config_item *item, char *page)
> > return snprintf(page, PAGE_SIZE, "%d\n",
> > to_mdev_link(item)->create);
> > }
> >
> > +static int set_config_and_add_link(struct mdev_link *mdev_link)
> > +{
> > + int i;
> > + int ret;
> > +
> > + for (i = 0; i < ARRAY_SIZE(set_config_val); i++) {
> > + ret = set_config_val[i](mdev_link);
> > + if (ret == -ENODATA) {
> I've read the commit description but I don't really understand the
> error codes. Here only -ENODATA is an error. But later -ENODEV
> is success. Why not "if (ret) {" here?
>
The most_set_cfg_* functions return success, ENODEV or ENODATA.
We want to stop only in case there is something wrong with the
provided data. A missing device is not an issue here. In this
case we want to keep the configuration as is and complete stuff
once a device is being attached.
>
> >
> > + pr_err("Config failed\n");
> > + return ret;
> > + }
> > + }
> > +
> > + return most_add_link(mdev_link->mdev, mdev_link->channel,
> > + mdev_link->comp, mdev_link->name,
> > + mdev_link->param);
> > +}
> > +
> > static ssize_t mdev_link_create_store(struct config_item *item,
> > const char *page, size_t
> > count)
> > {
> > struct mdev_link *mdev_link = to_mdev_link(item);
> > bool tmp;
> > int ret;
> > - int i;
> >
> > ret = kstrtobool(page, &tmp);
> > if (ret)
> > return ret;
> > -
> > - for (i = 0; i < ARRAY_SIZE(set_config_val); i++) {
> > - ret = set_config_val[i](mdev_link);
> > - if (ret < 0)
> > - return ret;
> > - }
> > -
> > - if (tmp)
> > - ret = most_add_link(mdev_link->mdev, mdev_link-
> > >channel,
> > - mdev_link->comp, mdev_link-
> > >name,
> > - mdev_link->param);
> > - else
> > - ret = most_remove_link(mdev_link->mdev, mdev_link-
> > >channel,
> > - mdev_link->comp);
> > - if (ret)
> > + if (!tmp)
> > + return most_remove_link(mdev_link->mdev,
> > mdev_link->channel,
> > + mdev_link->comp);
> > + ret = set_config_and_add_link(mdev_link);
> > + if (ret && ret != -ENODEV)
> ENODEV is success. I feel like this needs to be explained in
> comments
> in the code.
ENODEV is not a show-stopper. It only postpones the configuration
process until the driver is being notified about a new device.
>
> >
> > return ret;
> > + list_add_tail(&mdev_link->list, &mdev_link_list);
> > mdev_link->create = tmp;
> > return count;
> > }
> > @@ -590,6 +601,22 @@ int most_register_configfs_subsys(struct
> > core_component *c)
> > }
> > EXPORT_SYMBOL_GPL(most_register_configfs_subsys);
> >
> > +void most_interface_register_notify(const char *mdev)
> > +{
> > + bool register_snd_card = false;
> > + struct mdev_link *mdev_link;
> > +
> > + list_for_each_entry(mdev_link, &mdev_link_list, list) {
> > + if (!strcmp(mdev_link->mdev, mdev)) {
> > + set_config_and_add_link(mdev_link);
> Here the errors are not checked.
We are in the notify function. That means a device is present
now. And if the list isn't empty, there is also a valid configuration
available. So, set_config_and_add_link should not fail.
>
> >
> > + if (!strcmp
Re: [PATCH] staging: rtl8192u: fix incorrect mask for EEPROMTxPowerLevelCCK setting
On Fri, Mar 29, 2019 at 12:02:44AM +, Colin King wrote:
> From: Colin Ian King
>
> Currently the lower 8 bits of ret are being masked and left
> shifted by 8 bits always leaving a result of zero. The mask
> appears to be incorrect and should probably be 0xff00 instead
> of 0xff. Fix this. (Note: not tested).
>
> Fixes: 16feab644fd1 ("staging: rtl8192u: check return value eprom_read")
> Signed-off-by: Colin Ian King
> ---
> drivers/staging/rtl8192u/r8192U_core.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/staging/rtl8192u/r8192U_core.c
> b/drivers/staging/rtl8192u/r8192U_core.c
> index f1eaab337dca..a173884d31c8 100644
> --- a/drivers/staging/rtl8192u/r8192U_core.c
> +++ b/drivers/staging/rtl8192u/r8192U_core.c
> @@ -2454,7 +2454,7 @@ static int rtl8192_read_eeprom_info(struct net_device
> *dev)
> ret = eprom_read(dev, (EEPROM_TX_PW_INDEX_CCK
> >> 1));
> if (ret < 0)
> return ret;
> - priv->EEPROMTxPowerLevelCCK = ((u16)ret & 0xff)
> >> 8;
> + priv->EEPROMTxPowerLevelCCK = ((u16)ret &
> 0xff00) >> 8;
I'd say there is a 80-90% chance your fix is correct...
This only affects an older rev of the eeprom I think. I believe what
happens in the current code is that we set EEPROMTxPowerLevelCCK to
zero. Then we subtract:
priv->TxPowerLevelCCK[i] = priv->EEPROMTxPowerLevelOFDM24G[0] +
(priv->EEPROMTxPowerLevelCCK - priv->EEPROMTxPowerLevelOFDM24G[1]);
Possibly leading to a high u8 value, then in phy_set_rf8256_cck_tx_power()
it gets capped at 0x24...
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 3/3] staging: mt7621-pci-phy: change driver to don't use child nodes
Hi Dan,
On Fri, Mar 29, 2019 at 8:54 AM Dan Carpenter wrote:
>
> On Thu, Mar 28, 2019 at 06:55:31PM +0100, Sergio Paracuellos wrote:
> > static int mt7621_pci_phy_probe(struct platform_device *pdev)
> > {
> > struct device *dev = &pdev->dev;
> > @@ -299,6 +315,7 @@ static int mt7621_pci_phy_probe(struct platform_device
> > *pdev)
> > struct resource res;
> > int port, ret;
> > void __iomem *port_base;
> > + u32 phy_num;
> >
> > phy = devm_kzalloc(dev, sizeof(*phy), GFP_KERNEL);
> > if (!phy)
> > @@ -325,8 +342,9 @@ static int mt7621_pci_phy_probe(struct platform_device
> > *pdev)
> > return PTR_ERR(port_base);
> > }
> >
> > - port = 0;
> > - for_each_child_of_node(np, child_np) {
> > + of_property_read_u32(dev->of_node, "#phy-cells", &phy_num);
> > +
> > + for (port = 0; port < phy_num + 1; port++) {
>^^ ^^
> > struct mt7621_pci_phy_instance *instance;
> > struct phy *pphy;
> >
> > @@ -338,7 +356,7 @@ static int mt7621_pci_phy_probe(struct platform_device
> > *pdev)
> >
> > phy->phys[port] = instance;
> >
> > - pphy = devm_phy_create(dev, child_np, &mt7621_pci_phy_ops);
> > + pphy = devm_phy_create(dev, dev->of_node,
> > &mt7621_pci_phy_ops);
> > if (IS_ERR(phy)) {
> > dev_err(dev, "failed to create phy\n");
> > ret = PTR_ERR(phy);
> > @@ -352,7 +370,7 @@ static int mt7621_pci_phy_probe(struct platform_device
> > *pdev)
> > port++;
> ^^
> > }
>
> Incrementing "port" twice is probably wrong... Or anyway, less readable
> than "port += 2".
Yes, that was a mistake in my code becase I did not delete it when the
loop was changed.
>
> To be honest, I don't know anything about device tree. Does "phy_num"
> come from the device tree stuff that you just changed in the ealier
> patches? (I never read those so I never learn anything about device
> tree so I am stuck in an endless doom cycle).
The first approach in v1 was to read this from #phy-cell property from
device tree. Neil points me
out this was not the correct approach and was changed to a fixed
MAX_PHYS for both phy's in v2 patches.
>
> Anyway, I am a real newbie. Where does the plus one in
> "port < phy_num + 1" come from? How do I verify that phy_num is less
> than phy->nphys?
In the same way, this is now a fixed port < MAX_PHYS in for loop sent
in v2 of the patches.
>
> regards,
> dan carpenter
>
Best regards,
Sergio Paracuellos
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 3/3] staging: mt7621-pci-phy: change driver to don't use child nodes
On Thu, Mar 28, 2019 at 06:55:31PM +0100, Sergio Paracuellos wrote:
> static int mt7621_pci_phy_probe(struct platform_device *pdev)
> {
> struct device *dev = &pdev->dev;
> @@ -299,6 +315,7 @@ static int mt7621_pci_phy_probe(struct platform_device
> *pdev)
> struct resource res;
> int port, ret;
> void __iomem *port_base;
> + u32 phy_num;
>
> phy = devm_kzalloc(dev, sizeof(*phy), GFP_KERNEL);
> if (!phy)
> @@ -325,8 +342,9 @@ static int mt7621_pci_phy_probe(struct platform_device
> *pdev)
> return PTR_ERR(port_base);
> }
>
> - port = 0;
> - for_each_child_of_node(np, child_np) {
> + of_property_read_u32(dev->of_node, "#phy-cells", &phy_num);
> +
> + for (port = 0; port < phy_num + 1; port++) {
^^ ^^
> struct mt7621_pci_phy_instance *instance;
> struct phy *pphy;
>
> @@ -338,7 +356,7 @@ static int mt7621_pci_phy_probe(struct platform_device
> *pdev)
>
> phy->phys[port] = instance;
>
> - pphy = devm_phy_create(dev, child_np, &mt7621_pci_phy_ops);
> + pphy = devm_phy_create(dev, dev->of_node, &mt7621_pci_phy_ops);
> if (IS_ERR(phy)) {
> dev_err(dev, "failed to create phy\n");
> ret = PTR_ERR(phy);
> @@ -352,7 +370,7 @@ static int mt7621_pci_phy_probe(struct platform_device
> *pdev)
> port++;
^^
> }
Incrementing "port" twice is probably wrong... Or anyway, less readable
than "port += 2".
To be honest, I don't know anything about device tree. Does "phy_num"
come from the device tree stuff that you just changed in the ealier
patches? (I never read those so I never learn anything about device
tree so I am stuck in an endless doom cycle).
Anyway, I am a real newbie. Where does the plus one in
"port < phy_num + 1" come from? How do I verify that phy_num is less
than phy->nphys?
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
