Re: [Outreachy kernel] [PATCH] staging: android: ion: Fix parenthesis alignment
On Wed, 1 Apr 2020, John B. Wyatt IV wrote: > Fix 2 parenthesis alignment issues. Please try to find a way to describe what you have done that doesn't involve the word "Fix". What have you done and why? julia > > Reported by checkpatch. > > Signed-off-by: John B. Wyatt IV > --- > drivers/staging/android/ion/ion_page_pool.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/staging/android/ion/ion_page_pool.c > b/drivers/staging/android/ion/ion_page_pool.c > index f85ec5b16b65..0198b886d906 100644 > --- a/drivers/staging/android/ion/ion_page_pool.c > +++ b/drivers/staging/android/ion/ion_page_pool.c > @@ -37,7 +37,7 @@ static void ion_page_pool_add(struct ion_page_pool *pool, > struct page *page) > } > > mod_node_page_state(page_pgdat(page), NR_KERNEL_MISC_RECLAIMABLE, > - 1 << pool->order); > + 1 << pool->order); > mutex_unlock(&pool->mutex); > } > > @@ -57,7 +57,7 @@ static struct page *ion_page_pool_remove(struct > ion_page_pool *pool, bool high) > > list_del(&page->lru); > mod_node_page_state(page_pgdat(page), NR_KERNEL_MISC_RECLAIMABLE, > - -(1 << pool->order)); > + -(1 << pool->order)); > return page; > } > > -- > 2.25.1 > > -- > You received this message because you are subscribed to the Google Groups > "outreachy-kernel" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to outreachy-kernel+unsubscr...@googlegroups.com. > To view this discussion on the web visit > https://groups.google.com/d/msgid/outreachy-kernel/20200402012515.429329-1-jbwyatt4%40gmail.com. > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 4/5] staging: rtl8712: fix multiline derefernce warning
Hi Aiman, On Sat, Mar 28, 2020 at 12:17:19PM -0700, Joe Perches wrote: > On Fri, 2020-03-27 at 20:08 -0400, aimannajjar wrote: > > so these would be > > ether_addr_copy(pwlanhdr->addr2, pattr->src); > > and pwlanhdr isn't a particularly valuable name > for an automatic either. It's hungarian like. > "Hungarian like" means it starts with a "p". https://en.wikipedia.org/wiki/Hungarian_notation It's against the rules of kernel style. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: rtl8712: fix checkpatch warnings
On Thu, Mar 26, 2020 at 01:56:16AM -0400, Aiman Najjar wrote: > @@ -350,7 +351,7 @@ static int xmitframe_addmic(struct _adapter *padapter, > struct sta_info *stainfo; > struct qos_priv *pqospriv = &(padapter->mlmepriv.qospriv); > struct pkt_attrib *pattrib = &pxmitframe->attrib; > - struct security_priv *psecuritypriv = &padapter->securitypriv; > + struct security_priv *psecpriv = &padapter->securitypriv; This patch is doing too many things of course, but the other problem is that when you're renaming variables we don't what them to start with "p" to mean that they are a pointer. "psecpriv" should just be "secpriv". That name is still kind of rubbish, but it's not against the rules like starting with a p for pointer. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 1/5] staging: rtl8712: fix checkpatch long-line warning
On Fri, Mar 27, 2020 at 08:08:07PM -0400, aimannajjar wrote: > This patch fixes these two long-line checkpatch warnings > in rtl871x_xmit.c: > > WARNING: line over 80 characters > \#74: FILE: drivers/staging//rtl8712/rtl871x_xmit.c:74: > + * Please allocate memory with the sz = (struct xmit_frame) * > NR_XMITFRAME, > > WARNING: line over 80 characters > \#79: FILE: drivers/staging//rtl8712/rtl871x_xmit.c:79: > + kmalloc(NR_XMITFRAME * sizeof(struct xmit_frame) + 4, > GFP_ATOMIC); > > Signed-off-by: aimannajjar ^^^ You need to use your proper legal name here. Please capitalize normally like you would on a legal document. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 1/5] staging: rtl8712: fix checkpatch long-line warning
On Fri, Mar 27, 2020 at 08:08:07PM -0400, aimannajjar wrote: > This patch fixes these two long-line checkpatch warnings > in rtl871x_xmit.c: > > WARNING: line over 80 characters > \#74: FILE: drivers/staging//rtl8712/rtl871x_xmit.c:74: > + * Please allocate memory with the sz = (struct xmit_frame) * > NR_XMITFRAME, > > WARNING: line over 80 characters > \#79: FILE: drivers/staging//rtl8712/rtl871x_xmit.c:79: > + kmalloc(NR_XMITFRAME * sizeof(struct xmit_frame) + 4, > GFP_ATOMIC); > > Signed-off-by: aimannajjar > --- > drivers/staging/rtl8712/rtl871x_xmit.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/staging/rtl8712/rtl871x_xmit.c > b/drivers/staging/rtl8712/rtl871x_xmit.c > index f0b85338b567..628e4bad1547 100644 > --- a/drivers/staging/rtl8712/rtl871x_xmit.c > +++ b/drivers/staging/rtl8712/rtl871x_xmit.c > @@ -71,12 +71,13 @@ int _r8712_init_xmit_priv(struct xmit_priv *pxmitpriv, > _init_queue(&pxmitpriv->apsd_queue); > _init_queue(&pxmitpriv->free_xmit_queue); > /* > - * Please allocate memory with the sz = (struct xmit_frame) * > NR_XMITFRAME, > + * Please allocate memory with sz = (struct xmit_frame) * NR_XMITFRAME, >* and initialize free_xmit_frame below. >* Please also apply free_txobj to link_up all the xmit_frames... Probably you could delete the "Please ". regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v3] staging: wilc1000: Use crc7 in lib/ rather than a private copy
On Thu, Mar 26, 2020 at 03:23:36PM +, ajay.kat...@microchip.com wrote: > From: George Spelvin > > The code in lib/ is the desired polynomial, and even includes > the 1-bit left shift in the table rather than needing to code > it explicitly. > > While I'm in Kconfig, add a description of what a WILC1000 is. > Kconfig questions that require me to look up a data sheet to > find out that I probably don't have one are a pet peeve. > I don't know how this patch made it through two versions without anyone complaining that this paragraph should be done as a separate patch... > Cc: Adham Abozaeid > Cc: linux-wirel...@vger.kernel.org > Reviewed-by: Ajay Singh > Signed-off-by: George Spelvin > --- This should have you Signed-off-by. The Reviewed-by is kind of assumed so you can drop that bit. But everyone who touches a patch needs to add their signed off by. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 0/4] media Kconfig reorg - part 2
Em Wed, 1 Apr 2020 13:59:49 +0300 Dan Carpenter escreveu: > On Wed, Mar 25, 2020 at 04:36:31PM -0300, Helen Koike wrote: > > Hello, > > > > On 3/25/20 1:03 PM, Mauro Carvalho Chehab wrote: > > > That's the second part of media Kconfig changes. The entire series is > > > at: > > > > > > https://git.linuxtv.org/mchehab/experimental.git/log/?h=media-kconfig > > > > I made a quick experiment (using this branch) with someone who works > > with the kernel for his master degree, but doesn't have much experience in > > kernel development in general. > > I asked him to enable Vimc (from default configs, where multimedia starts > > disabled). > > The whole config system is really outdated. Agreed. Btw, when compiled against Qt 5.14, "make xconfig" is currently broken. I'm sending in a few some fixup patches for it. > It should be that this task was done with a command like "kconfig enable > vimc". It would ask necessary questions and pull in the dependencies > automatically. Yes. That's something that it is missing for a long time. There were some efforts to add a SAT solver at the Kernel that could be used for that, but I dunno what's current status. > Twenty years ago it made sense to go through the menus and select things > one by one. Does anyone really start from defconfig any more? Surely > everyone starts with a known working config and just enables specific > options. Yeah, that's my feeling too. > I started to hack together some code to create a kconfig program to > enable and disable options. The problem is that all library code > assumes we want to display menus so it was a lot of work and I gave up. :-( Thanks, Mauro ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: vt6656: Use defines in vnt_mac_reg_bits_* functions
On 04/01/20 18:55:38, Oscar Carter wrote:
> On Tue, Mar 31, 2020 at 01:29:06PM +0300, Dan Carpenter wrote:
> > On Sat, Mar 28, 2020 at 10:54:33AM +0100, Oscar Carter wrote:
> > > Define the necessary bits in the CHANNEL, PAPEDELAY and GPIOCTL0
> > > registers to can use them in the calls to vnt_mac_reg_bits_on and
> > > vnt_mac_reg_bits_off functions. In this way, avoid the use of BIT()
> > > macros and clarify the code.
> > >
> > > Fixes: 3017e587e368 ("staging: vt6656: Use BIT() macro in
> > > vnt_mac_reg_bits_* functions")
> > > Suggested-by: Dan Carpenter
> > > Signed-off-by: Oscar Carter
> > > ---
> > > drivers/staging/vt6656/baseband.c | 6 --
> > > drivers/staging/vt6656/card.c | 3 +--
> > > drivers/staging/vt6656/mac.h | 12
> > > drivers/staging/vt6656/main_usb.c | 2 +-
> > > 4 files changed, 18 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/drivers/staging/vt6656/baseband.c
> > > b/drivers/staging/vt6656/baseband.c
> > > index a19a563d8bcc..dd3c3bf5e8b5 100644
> > > --- a/drivers/staging/vt6656/baseband.c
> > > +++ b/drivers/staging/vt6656/baseband.c
> > > @@ -442,7 +442,8 @@ int vnt_vt3184_init(struct vnt_private *priv)
> > > if (ret)
> > > goto end;
> > >
> > > - ret = vnt_mac_reg_bits_on(priv, MAC_REG_PAPEDELAY, BIT(0));
> > > + ret = vnt_mac_reg_bits_on(priv, MAC_REG_PAPEDELAY,
> > > + PAPEDELAY_B0);
> >
> > This doesn't clarify anything. It makes it less clear because someone
> > would assume B0 means something but it's just hiding a magic number
> > behind a meaningless define. B0 means BIT(0) which means nothing. So
> > now we have to jump through two hoops to find out that we don't know
> > anything.
> >
> I created this names due to the lack of information about the hardware. I
> searched but I did not find anything.
>
> > Just leave it as-is. Same for the rest.
> Ok.
>
> >
> > There problem is a hardware spec which explains what this stuff is.
> >
> It's possible to find a datasheet of this hardware to make this modification
> accordingly to the correct bit names of registers ?
I haven't found any so far, if your researches are luckier than mine, I
would be interested too. Even getting your hands on the actual device is
complicated.
Thanks,
Quentin
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2] staging: vt6656: add error code handling to unused variable
Ignore this one. John sent it by mistake and has already sent a v3. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: rtl8188eu: refactor Efuse_GetCurrentSize()
On Sun, Mar 29, 2020 at 12:04:50PM +0200, Michael Straube wrote: > Refactor while loop in Efuse_GetCurrentSize() to reduce indentation > level and clear line over 80 characters checkpatch warnings. > > Signed-off-by: Michael Straube Reviewed-by: Dan Carpenter regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v3 0/5] staging: rtl8712: rtl871x_xmit.{c, h} code style improvements
Looks good. Thanks! Reviewed-by: Dan Carpenter regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 01/01] staging: gasket: Fix incongruency in handling of sysfs entries creation
On Sun, Mar 29, 2020 at 10:59:21PM +0100, Luís Mendes wrote:
> Fix incongruency in handling of sysfs entries creation.
> This issue could cause invalid memory accesses, by not properly
> detecting the end of the sysfs attributes array.
>
Please add a Fixes tag.
Fixes: 84c45d5f3bf1 ("staging: gasket: Replace macro __ATTR with __ATTR_NULL")
That patch was never sent to the proper mailing list for review.
Anyway, Luis, you will need to resend because your patch doesn't apply.
Please read the first paragraphs of Documentation/process/email-clients.rst
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] binderfs: remove redundant assignment to pointer ctx
From: Colin Ian King
The pointer ctx is being initialized with a value that is never read
and it is being updated later with a new value. The initialization
is redundant and can be removed.
Addresses-Coverity: ("Unused value")
Signed-off-by: Colin Ian King
---
drivers/android/binderfs.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/android/binderfs.c b/drivers/android/binderfs.c
index 9ecad74183a3..8352a3d160bf 100644
--- a/drivers/android/binderfs.c
+++ b/drivers/android/binderfs.c
@@ -747,7 +747,7 @@ static const struct fs_context_operations
binderfs_fs_context_ops = {
static int binderfs_init_fs_context(struct fs_context *fc)
{
- struct binderfs_mount_opts *ctx = fc->fs_private;
+ struct binderfs_mount_opts *ctx;
ctx = kzalloc(sizeof(struct binderfs_mount_opts), GFP_KERNEL);
if (!ctx)
--
2.25.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: vt6656: Use defines in vnt_mac_reg_bits_* functions
On 02/04/2020 10:19, Quentin Deslandes wrote:
On 04/01/20 18:55:38, Oscar Carter wrote:
On Tue, Mar 31, 2020 at 01:29:06PM +0300, Dan Carpenter wrote:
On Sat, Mar 28, 2020 at 10:54:33AM +0100, Oscar Carter wrote:
Define the necessary bits in the CHANNEL, PAPEDELAY and GPIOCTL0
registers to can use them in the calls to vnt_mac_reg_bits_on and
vnt_mac_reg_bits_off functions. In this way, avoid the use of BIT()
macros and clarify the code.
Fixes: 3017e587e368 ("staging: vt6656: Use BIT() macro in vnt_mac_reg_bits_*
functions")
Suggested-by: Dan Carpenter
Signed-off-by: Oscar Carter
---
drivers/staging/vt6656/baseband.c | 6 --
drivers/staging/vt6656/card.c | 3 +--
drivers/staging/vt6656/mac.h | 12
drivers/staging/vt6656/main_usb.c | 2 +-
4 files changed, 18 insertions(+), 5 deletions(-)
diff --git a/drivers/staging/vt6656/baseband.c
b/drivers/staging/vt6656/baseband.c
index a19a563d8bcc..dd3c3bf5e8b5 100644
--- a/drivers/staging/vt6656/baseband.c
+++ b/drivers/staging/vt6656/baseband.c
@@ -442,7 +442,8 @@ int vnt_vt3184_init(struct vnt_private *priv)
if (ret)
goto end;
- ret = vnt_mac_reg_bits_on(priv, MAC_REG_PAPEDELAY, BIT(0));
+ ret = vnt_mac_reg_bits_on(priv, MAC_REG_PAPEDELAY,
+ PAPEDELAY_B0);
This doesn't clarify anything. It makes it less clear because someone
would assume B0 means something but it's just hiding a magic number
behind a meaningless define. B0 means BIT(0) which means nothing. So
now we have to jump through two hoops to find out that we don't know
anything.
I created this names due to the lack of information about the hardware. I
searched but I did not find anything.
Just leave it as-is. Same for the rest.
Ok.
There problem is a hardware spec which explains what this stuff is.
It's possible to find a datasheet of this hardware to make this modification
accordingly to the correct bit names of registers ?
I haven't found any so far, if your researches are luckier than mine, I
would be interested too. Even getting your hands on the actual device is
complicated.
All I can tell you is it related to command above it MAC_REG_ITRTMSET
without it the device will not associate with access point is probably
TX timing/power rated.
Other bits in MAC_REG_PAPEDELAY are related to LED function and defined
in LEDSTS_* in mac.h.
I think it is some kind of enable so something like ITRTMSET_ENABLE
would do.
Regards
Malcolm
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] binderfs: remove redundant assignment to pointer ctx
On Thu, Apr 02, 2020 at 11:50:00AM +0100, Colin King wrote:
> From: Colin Ian King
>
> The pointer ctx is being initialized with a value that is never read
> and it is being updated later with a new value. The initialization
> is redundant and can be removed.
>
> Addresses-Coverity: ("Unused value")
> Signed-off-by: Colin Ian King
Acked-by: Christian Brauner
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[driver-core:debugfs_cleanup] BUILD SUCCESS 71280bf8796b28d6fc09ea490276d02e240948b5
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/driver-core.git debugfs_cleanup branch HEAD: 71280bf8796b28d6fc09ea490276d02e240948b5 powerpc: powernv: no need to check return value of debugfs_create functions elapsed time: 1322m configs tested: 162 configs skipped: 0 The following configs have been built successfully. More configs may be tested in the coming days. arm64allmodconfig arm64 allnoconfig arm allnoconfig arm64allyesconfig arm allyesconfig arm efm32_defconfig arm at91_dt_defconfig armshmobile_defconfig arm64 defconfig arm exynos_defconfig armmulti_v5_defconfig arm sunxi_defconfig armmulti_v7_defconfig sparcallyesconfig m68k m5475evb_defconfig i386 allnoconfig i386 allyesconfig i386 alldefconfig i386defconfig ia64 allmodconfig ia64defconfig ia64 allnoconfig ia64 allyesconfig ia64 alldefconfig arm allmodconfig nios2 3c120_defconfig nios2 10m50_defconfig c6xevmc6678_defconfig xtensa iss_defconfig c6x allyesconfig xtensa common_defconfig openrisc simple_smp_defconfig openriscor1ksim_defconfig nds32 defconfig nds32 allnoconfig cskydefconfig alpha defconfig h8300 h8s-sim_defconfig h8300 edosk2674_defconfig m68k allmodconfig h8300h8300h-sim_defconfig m68k sun3_defconfig m68k multi_defconfig arc defconfig arc allyesconfig powerpc defconfig powerpc ppc64_defconfig microblaze mmu_defconfig microblazenommu_defconfig powerpc allnoconfig mips fuloong2e_defconfig mips malta_kvm_defconfig mips allyesconfig mips 64r6el_defconfig mips allnoconfig mips 32r2_defconfig mips allmodconfig pariscallnoconfig pariscgeneric-64bit_defconfig pariscgeneric-32bit_defconfig parisc allyesconfig x86_64 randconfig-a003-20200401 i386 randconfig-a002-20200401 x86_64 randconfig-a002-20200401 x86_64 randconfig-a001-20200401 i386 randconfig-a003-20200401 i386 randconfig-a001-20200401 mips randconfig-a001-20200401 nds32randconfig-a001-20200401 m68k randconfig-a001-20200401 alpharandconfig-a001-20200401 parisc randconfig-a001-20200401 riscvrandconfig-a001-20200401 sparc64 randconfig-a001-20200401 h8300randconfig-a001-20200401 nios2randconfig-a001-20200401 microblaze randconfig-a001-20200401 c6x randconfig-a001-20200401 csky randconfig-a001-20200401 openrisc randconfig-a001-20200401 s390 randconfig-a001-20200401 sh randconfig-a001-20200401 xtensa randconfig-a001-20200401 i386 randconfig-b003-20200401 x86_64 randconfig-b002-20200401 x86_64 randconfig-b003-20200401 i386 randconfig-b001-20200401 x86_64 randconfig-b001-20200401 i386 randconfig-b002-20200401 i386 randconfig-c001-20200401 i386 randconfig-c003-20200401 x86_64 randconfig-c003-20200401 i386 randconfig-c002-20200401 x86_64 randconfig-c001-20200401 x86_64 randconfig-c002-20200401 x86_64 randconfig-d001-20200401 x86_64 randconfig-d002-20200401 x86_64 randconfig-d003-20200401 i386 randconfig-d001-20200401 i386 randconfig-d002-20200401 i3
Re: [Update PATCH] x86/Hyper-V: Initialize Syn timer clock when it's
This doesn't apply to today's linux-next. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/4] dt-bindings: media: rkisp1: move rockchip-isp1 bindings out of staging
Hi Helen,
> # SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> %YAML 1.2
> ---
> $id: http://devicetree.org/schemas/media/rockchip-isp1.yaml#
> $schema: http://devicetree.org/meta-schemas/core.yaml#
>
> title: Rockchip SoC Image Signal Processing unit v1
Where do we need 'v1' for? Is there a 'v2'?
>
> maintainers:
> - Helen Koike
>
> description: |
> Rockchip ISP1 is the Camera interface for the Rockchip series of SoCs
> which contains image processing, scaling, and compression functions.
>
> properties:
> compatible:
> const: rockchip,rk3399-cif-isp
>
> reg:
> maxItems: 1
>
> interrupts:
> maxItems: 1
>
> iommus:
> maxItems: 1
>
> power-domains:
> maxItems: 1
>
> phys:
> maxItems: 1
> description: phandle for the PHY port
>
> phy-names:
> const: dphy
>
> clocks:
> items:
> - description: ISP clock
> - description: ISP AXI clock clock
> - description: ISP AXI clock wrapper clock
> - description: ISP AHB clock clock
> - description: ISP AHB wrapper clock
>
> clock-names:
> items:
> - const: clk_isp
> - const: aclk_isp
> - const: aclk_isp_wrap
> - const: hclk_isp
> - const: hclk_isp_wrap
>
> # See ./video-interfaces.txt for details
> ports:
> type: object
> additionalProperties: false
>
> properties:
> "#address-cells":
> const: 1
>
> "#size-cells":
> const: 0
>
> port@0:
> type: object
> description: connection point for sensors at MIPI-DPHY RX0
> additionalProperties: false
Nothing required here?
>
> properties:
> "#address-cells":
> const: 1
>
> "#size-cells":
> const: 0
>
> reg:
> const: 0
>
> patternProperties:
> endpoint:
> type: object
> additionalProperties: false
>
> properties:
> reg:
> maxItems: 1
>
> data-lanes:
> minItems: 1
> maxItems: 4
>
> remote-endpoint: true
>
> required:
> - port@0
The use of '@0' makes "#address-cells" and "#size-cells" also a requirement.
- "#address-cells"
- "#size-cells"
>
> required:
> - compatible
How about 'reg'?
- reg
> - interrupts
> - clocks
> - clock-names
> - power-domains
> - iommus
> - phys
> - phy-names
> - ports
>
> additionalProperties: false
>
> examples:
> - |
>
> #include
> #include
> #include
>
> parent0: parent@0 {
> #address-cells = <2>;
> #size-cells = <2>;
>
> isp0: isp0@ff91 {
> compatible = "rockchip,rk3399-cif-isp";
> reg = <0x0 0xff91 0x0 0x4000>;
> interrupts = ;
> clocks = <&cru SCLK_ISP0>,
> <&cru ACLK_ISP0>, <&cru ACLK_ISP0_WRAPPER>,
> <&cru HCLK_ISP0>, <&cru HCLK_ISP0_WRAPPER>;
> clock-names = "clk_isp",
> "aclk_isp", "aclk_isp_wrap",
> "hclk_isp", "hclk_isp_wrap";
> power-domains = <&power RK3399_PD_ISP0>;
> iommus = <&isp0_mmu>;
> phys = <&dphy>;
> phy-names = "dphy";
>
> ports {
> #address-cells = <1>;
> #size-cells = <0>;
>
> port@0 {
> #address-cells = <1>;
> #size-cells = <0>;
> reg = <0>;
>
> mipi_in_wcam: endpoint@0 {
> reg = <0>;
> remote-endpoint = <&wcam_out>;
> data-lanes = <1 2>;
> };
>
> mipi_in_ucam: endpoint@1 {
> reg = <1>;
> remote-endpoint = <&ucam_out>;
> data-lanes = <1>;
> };
> };
> };
> };
>
> i2c7: i2c@ff16 {
> clock-frequency = <40>;
> #address-cells = <1>;
> #size-cells = <0>;
Incomplete example.
>From i2c-rk3x.yaml:
required:
- compatible
- reg
- interrupts
- clocks
- clock-names
>
> wcam: camera@36 {
> compatible = "ovti,ov5695";
> reg = <0x36>;
>
> port {
> wcam_out: endpoint {
> remote-endpoint = <&mipi_in_wcam>;
> data-lanes = <1 2>;
> };
> };
> };
>
> ucam: camera@3c {
> compatible = "ovti,ov2685";
> reg = <0x3c>;
>
> port {
> ucam_out: endpoint {
> remote-endpoint = <&mipi_in_ucam>;
> data-lanes = <1>;
> }
[driver-core:debugfs_remove_return_value] BUILD SUCCESS 927e420044b36db8b8be2ed95e7b723b7808ebe4
randconfig-e001-20200401 x86_64 randconfig-e002-20200401 i386 randconfig-e003-20200401 x86_64 randconfig-e001-20200401 i386 randconfig-e002-20200401 i386 randconfig-f001-20200401 i386 randconfig-f003-20200401 x86_64 randconfig-f003-20200401 x86_64 randconfig-f001-20200401 i386 randconfig-f002-20200401 x86_64 randconfig-f002-20200401 x86_64 randconfig-g003-20200401 i386 randconfig-g003-20200401 x86_64 randconfig-g002-20200401 i386 randconfig-g001-20200401 i386 randconfig-g002-20200401 x86_64 randconfig-g001-20200401 x86_64 randconfig-h001-20200402 x86_64 randconfig-h002-20200402 x86_64 randconfig-h003-20200402 i386 randconfig-h001-20200402 i386 randconfig-h002-20200402 i386 randconfig-h003-20200402 x86_64 randconfig-h002-20200401 i386 randconfig-h002-20200401 i386 randconfig-h003-20200401 i386 randconfig-h001-20200401 x86_64 randconfig-h001-20200401 x86_64 randconfig-h003-20200401 arc randconfig-a001-20200401 arm randconfig-a001-20200401 arm64randconfig-a001-20200401 ia64 randconfig-a001-20200401 powerpc randconfig-a001-20200401 sparcrandconfig-a001-20200401 riscvallyesconfig riscvnommu_virt_defconfig riscv allnoconfig riscv defconfig riscv rv32_defconfig riscvallmodconfig s390 zfcpdump_defconfig s390 debug_defconfig s390 allyesconfig s390 allnoconfig s390 allmodconfig s390 alldefconfig s390defconfig sh rsk7269_defconfig sh allmodconfig shtitan_defconfig sh sh7785lcr_32bit_defconfig shallnoconfig sparc defconfig sparc64 allmodconfig sparc64 allnoconfig sparc64 allyesconfig sparc64 defconfig um x86_64_defconfig um i386_defconfig um defconfig x86_64 rhel x86_64 rhel-7.6 x86_64 rhel-7.2-clear x86_64lkp x86_64 fedora-25 x86_64 kexec --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/4] dt-bindings: phy: phy-rockchip-dphy-rx0: move rockchip dphy rx0 bindings out of staging
Hi Helen,
> # SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> %YAML 1.2
> ---
> $id: http://devicetree.org/schemas/phy/rockchip-mipi-dphy-rx0.yaml#
> $schema: http://devicetree.org/meta-schemas/core.yaml#
>
> title: Rockchip SoC MIPI RX0 D-PHY Device Tree Bindings
>
> maintainers:
> - Helen Koike
> - Ezequiel Garcia
>
> description: |
> The Rockchip SoC has a MIPI D-PHY bus with an RX0 entry which connects to
> the ISP1 (Image Signal Processing unit v1.0) for CSI cameras.
>
> properties:
> compatible:
> const: rockchip,rk3399-mipi-dphy-rx0
>
> reg:
> maxItems: 1
If 'reg' is not used => remove it.
>
> clocks:
> items:
> - description: MIPI D-PHY ref clock
> - description: MIPI D-PHY RX0 cfg clock
> - description: Video in/out general register file clock
>
> clock-names:
> items:
> - const: dphy-ref
> - const: dphy-cfg
> - const: grf
>
> '#phy-cells':
> const: 0
>
> power-domains:
> description: Video in/out power domain.
> maxItems: 1
>
> required:
> - compatible
> - clocks
> - clock-names
> - '#phy-cells'
> - power-domains
>
> additionalProperties: false
>
> examples:
> - |
>
> /*
> * MIPI D-PHY RX0 use registers in "general register files", it
> * should be a child of the GRF.
> *
> * grf: syscon@ff77 {
> * compatible = "rockchip,rk3399-grf", "syscon", "simple-mfd";
> * ...
> * };
> */
>
> #include
> #include
>
> mipi_dphy_rx0: mipi-dphy-rx0 {
> compatible = "rockchip,rk3399-mipi-dphy-rx0";
> clocks = <&cru SCLK_MIPIDPHY_REF>,
> <&cru SCLK_DPHY_RX0_CFG>,
> <&cru PCLK_VIO_GRF>;
> clock-names = "dphy-ref", "dphy-cfg", "grf";
> power-domains = <&power RK3399_PD_VIO>;
> #phy-cells = <0>;
> };
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH -next] staging: greybus: fix a missing-check bug in gb_lights_light_config()
On Wed, Apr 01, 2020 at 11:00:17AM +0800, Chen Zhou wrote: > In gb_lights_light_config(), 'light->name' is allocated by kstrndup(). > It returns NULL when fails, add check for it. > > Signed-off-by: Chen Zhou > --- > drivers/staging/greybus/light.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/staging/greybus/light.c b/drivers/staging/greybus/light.c > index d6ba25f..d2672b6 100644 > --- a/drivers/staging/greybus/light.c > +++ b/drivers/staging/greybus/light.c > @@ -1026,7 +1026,8 @@ static int gb_lights_light_config(struct gb_lights > *glights, u8 id) > > light->channels_count = conf.channel_count; > light->name = kstrndup(conf.name, NAMES_MAX, GFP_KERNEL); > - > + if (!light->name) > + return -ENOMEM; > light->channels = kcalloc(light->channels_count, > sizeof(struct gb_channel), GFP_KERNEL); > if (!light->channels) The clean up in this function is non-existant. :( regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [Update PATCH] x86/Hyper-V: Initialize Syn timer clock when it's
Hi Dan： Sorry. Please ignore this patch and it's based on the old code. On 4/2/2020 7:21 PM, Dan Carpenter wrote: This doesn't apply to today's linux-next. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 01/32] staging: wfx: add sanity checks to hif_join()
On Wed, Apr 01, 2020 at 01:03:34PM +0200, Jerome Pouiller wrote: > From: Jérôme Pouiller > > Add a few check on start of hif_join(). > > Signed-off-by: Jérôme Pouiller > --- > drivers/staging/wfx/hif_tx.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/staging/wfx/hif_tx.c b/drivers/staging/wfx/hif_tx.c > index 77bca43aca42..445906035e9d 100644 > --- a/drivers/staging/wfx/hif_tx.c > +++ b/drivers/staging/wfx/hif_tx.c > @@ -297,6 +297,8 @@ int hif_join(struct wfx_vif *wvif, const struct > ieee80211_bss_conf *conf, > struct hif_req_join *body = wfx_alloc_hif(sizeof(*body), &hif); We've got an allocation here. It's a mistake to put the allocation in the declaration block because you're going to forget to check for failure. > > WARN_ON(!conf->basic_rates); > + WARN_ON(sizeof(body->ssid) < ssidlen); Put the variable on the left. WARN_ON(ssidlen > sizeof(body->ssid)). I'm not a big fan of adding this sort of debug code, just audit the callers to see if it's possible or not. I have audited the caller for you, and I believe that this condition *is possible* so we need to return -EINVAL in this situation to prevent memory corruption. if (ssidlen > sizeof(body->ssid)) return -EINVAL; > + WARN(!conf->ibss_joined && !ssidlen, "joining an unknown BSS"); > body->infrastructure_bss_mode = !conf->ibss_joined; ^ Potential NULL dererefence because of the unchecked allocation. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 04/32] staging: wfx: remove "burst" mechanism
On Wed, Apr 01, 2020 at 01:03:37PM +0200, Jerome Pouiller wrote:
> From: Jérôme Pouiller
>
> In the old days, the driver tried to reorder frames in order to send
> frames from the same queue grouped to the firmware. However, the
> firmware is able to do the job internally for a long time. There is no
> reasons to keep this mechanism.
>
> Signed-off-by: Jérôme Pouiller
> ---
> drivers/staging/wfx/queue.c | 23 ---
> drivers/staging/wfx/sta.c | 2 --
> drivers/staging/wfx/wfx.h | 1 -
> 3 files changed, 26 deletions(-)
>
> diff --git a/drivers/staging/wfx/queue.c b/drivers/staging/wfx/queue.c
> index e3aa1e346c70..712ac783514b 100644
> --- a/drivers/staging/wfx/queue.c
> +++ b/drivers/staging/wfx/queue.c
> @@ -363,8 +363,6 @@ static bool hif_handle_tx_data(struct wfx_vif *wvif,
> struct sk_buff *skb,
> static int wfx_get_prio_queue(struct wfx_vif *wvif,
>u32 tx_allowed_mask, int *total)
> {
> - static const int urgent = BIT(WFX_LINK_ID_AFTER_DTIM) |
> - BIT(WFX_LINK_ID_UAPSD);
> const struct ieee80211_tx_queue_params *edca;
> unsigned int score, best = -1;
^
Not related to this this patch but this confused me initially. UINT_MAX
would be more readable.
The other unrelated question I had about this function was:
402 /* search for a winner using edca params */
403 for (i = 0; i < IEEE80211_NUM_ACS; ++i) {
^
IEEE80211_NUM_ACS is 4.
404 int queued;
405
406 edca = &wvif->edca_params[i];
407 queued =
wfx_tx_queue_get_num_queued(&wvif->wdev->tx_queue[i],
408 tx_allowed_mask);
409 if (!queued)
410 continue;
411 *total += queued;
412 score = ((edca->aifs + edca->cw_min) << 16) +
413 ((edca->cw_max - edca->cw_min) *
414 (get_random_int() & 0x));
415 if (score < best && (winner < 0 || i != 3)) {
^^
Why do we not want winner to be 3? It's unrelated to the patch but
there should be a comment next to that code probably.
416 best = score;
417 winner = i;
418 }
419 }
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 08/32] staging: wfx: simplify hif_handle_tx_data()
On Wed, Apr 01, 2020 at 01:03:41PM +0200, Jerome Pouiller wrote:
> From: Jérôme Pouiller
>
> The last argument of hif_handle_tx_data() was now unused. In add,
> hif_handle_tx_data() has nothing to do with HIF layer and should be
> renamed. Finally, it not convenient to pass a wfx_vif as parameter. It
> is easier to let hif_handle_tx_data() find the interface itself.
>
> Signed-off-by: Jérôme Pouiller
> ---
> drivers/staging/wfx/queue.c | 19 ++-
> 1 file changed, 10 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/staging/wfx/queue.c b/drivers/staging/wfx/queue.c
> index 2553f77522d9..8647731e02c0 100644
> --- a/drivers/staging/wfx/queue.c
> +++ b/drivers/staging/wfx/queue.c
> @@ -319,13 +319,17 @@ bool wfx_tx_queues_is_empty(struct wfx_dev *wdev)
> return ret;
> }
>
> -static bool hif_handle_tx_data(struct wfx_vif *wvif, struct sk_buff *skb,
> -struct wfx_queue *queue)
> +static bool wfx_handle_tx_data(struct wfx_dev *wdev, struct sk_buff *skb)
> {
> struct hif_req_tx *req = wfx_skb_txreq(skb);
> struct ieee80211_key_conf *hw_key = wfx_skb_tx_priv(skb)->hw_key;
> struct ieee80211_hdr *frame =
> (struct ieee80211_hdr *)(req->frame +
> req->data_flags.fc_offset);
> + struct wfx_vif *wvif =
> + wdev_to_wvif(wdev, ((struct hif_msg *)skb->data)->interface);
^
This is on the TX side so it's probably okay, but one problem I have
noticed is that we do this on the RX side as well with checking that
if (skb->len < sizeof(struct hif_msg))
return -EINVAL;
So we could be reading beyond the end of the skb. If we got really
unlucky it could lead to an Oops.
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH -next] staging: greybus: fix a missing-check bug in gb_lights_light_config()
Hi Dan, On Thu, Apr 02, 2020 at 03:22:28PM +0300, Dan Carpenter wrote: > On Wed, Apr 01, 2020 at 11:00:17AM +0800, Chen Zhou wrote: > > In gb_lights_light_config(), 'light->name' is allocated by kstrndup(). > > It returns NULL when fails, add check for it. > > > > Signed-off-by: Chen Zhou > > --- > > drivers/staging/greybus/light.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/staging/greybus/light.c > > b/drivers/staging/greybus/light.c > > index d6ba25f..d2672b6 100644 > > --- a/drivers/staging/greybus/light.c > > +++ b/drivers/staging/greybus/light.c > > @@ -1026,7 +1026,8 @@ static int gb_lights_light_config(struct gb_lights > > *glights, u8 id) > > > > light->channels_count = conf.channel_count; > > light->name = kstrndup(conf.name, NAMES_MAX, GFP_KERNEL); > > - > > + if (!light->name) > > + return -ENOMEM; > > light->channels = kcalloc(light->channels_count, > > sizeof(struct gb_channel), GFP_KERNEL); > > if (!light->channels) > > The clean up in this function is non-existant. :( Yeah, this have a central point to do the cleanups, gb_lights_release, since we may have other lights already configured at this point, we could cleanup this specific one here, but than would need to make sure all other already configure got clean also. -- Cheers, Rui ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v3] staging: wilc1000: Use crc7 in lib/ rather than a private copy
Hi Dan, On 02/04/20 1:57 pm, Dan Carpenter wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the > content is safe > > On Thu, Mar 26, 2020 at 03:23:36PM +, ajay.kat...@microchip.com wrote: >> From: George Spelvin >> >> The code in lib/ is the desired polynomial, and even includes >> the 1-bit left shift in the table rather than needing to code >> it explicitly. >> >> While I'm in Kconfig, add a description of what a WILC1000 is. >> Kconfig questions that require me to look up a data sheet to >> find out that I probably don't have one are a pet peeve. >> > > I don't know how this patch made it through two versions without anyone > complaining that this paragraph should be done as a separate patch... > The first two version of patches were not submitted to devel@driverdev mailing list. I too missed the point to keep the Kconfig changes in separate patch. >> Cc: Adham Abozaeid >> Cc: linux-wirel...@vger.kernel.org >> Reviewed-by: Ajay Singh >> Signed-off-by: George Spelvin >> --- > > This should have you Signed-off-by. The Reviewed-by is kind of assumed > so you can drop that bit. But everyone who touches a patch needs to > add their signed off by. > Thanks, I will make a note of it. Regards Ajay ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] MAINTAINERS: remove entry after hp100 driver removal
Commit a10079c66290 ("staging: remove hp100 driver") removed all files
from ./drivers/staging/hp/, but missed to adjust MAINTAINERS.
Since then, ./scripts/get_maintainer.pl --self-test=patterns complains:
warning: no file matches F: drivers/staging/hp/hp100.*
So, drop HP100 Driver entry in MAINTAINERS now.
Signed-off-by: Lukas Bulwahn
---
Greg, here is a minor non-urgent patch for staging.
MAINTAINERS | 5 -
1 file changed, 5 deletions(-)
diff --git a/MAINTAINERS b/MAINTAINERS
index be43f1e37902..1c1abe8229af 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7721,11 +7721,6 @@ L: platform-driver-...@vger.kernel.org
S: Orphan
F: drivers/platform/x86/tc1100-wmi.c
-HP100: Driver for HP 10/100 Mbit/s Voice Grade Network Adapter Series
-M: Jaroslav Kysela
-S: Obsolete
-F: drivers/staging/hp/hp100.*
-
HPET: High Precision Event Timers driver
M: Clemens Ladisch
S: Maintained
--
2.17.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 3/4] arm64: dts: rockchip: add rx0 mipi-phy for rk3399
Hi Helen,
> From: Helen Koike
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> index 33cc21fcf4c10..fc0295d2a65a1 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> @@ -1394,6 +1394,17 @@ io_domains: io-domains {
> status = "disabled";
> };
>
> + mipi_dphy_rx0: mipi-dphy-rx0 {
For Heiko sort syscon@ff77 subnodes alphabetical or reg value first?
> + compatible = "rockchip,rk3399-mipi-dphy-rx0";
> + clocks = <&cru SCLK_MIPIDPHY_REF>,
> + <&cru SCLK_DPHY_RX0_CFG>,
> + <&cru PCLK_VIO_GRF>;
Align^
> + clock-names = "dphy-ref", "dphy-cfg", "grf";
> + power-domains = <&power RK3399_PD_VIO>;
> + #phy-cells = <0>;
> + status = "disabled";
> + };
> +
> u2phy0: usb2-phy@e450 {
> compatible = "rockchip,rk3399-usb2phy";
> reg = <0xe450 0x10>;
> --
> 2.26.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 01/01] staging: gasket: Fix incongruency in handling of sysfs entries creation
Hi Dan,
Ah sorry, ok, I will re-send the patch using git.
Can you please tell me which is the correct mailing list to where this
patch should be submitted?
Thanks,
Luís
On Thu, Apr 2, 2020 at 11:42 AM Dan Carpenter wrote:
>
> On Sun, Mar 29, 2020 at 10:59:21PM +0100, Luís Mendes wrote:
> > Fix incongruency in handling of sysfs entries creation.
> > This issue could cause invalid memory accesses, by not properly
> > detecting the end of the sysfs attributes array.
> >
>
> Please add a Fixes tag.
>
> Fixes: 84c45d5f3bf1 ("staging: gasket: Replace macro __ATTR with __ATTR_NULL")
>
> That patch was never sent to the proper mailing list for review.
>
> Anyway, Luis, you will need to resend because your patch doesn't apply.
> Please read the first paragraphs of Documentation/process/email-clients.rst
>
> regards,
> dan carpenter
>
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH -next] staging: greybus: fix a missing-check bug in gb_lights_light_config()
On Thu, Apr 02, 2020 at 02:16:18PM +0100, Rui Miguel Silva wrote: > > > --- a/drivers/staging/greybus/light.c > > > +++ b/drivers/staging/greybus/light.c > > > @@ -1026,7 +1026,8 @@ static int gb_lights_light_config(struct gb_lights > > > *glights, u8 id) > > > > > > light->channels_count = conf.channel_count; > > > light->name = kstrndup(conf.name, NAMES_MAX, GFP_KERNEL); > > > - > > > + if (!light->name) > > > + return -ENOMEM; > > > light->channels = kcalloc(light->channels_count, > > > sizeof(struct gb_channel), GFP_KERNEL); > > > if (!light->channels) > > > > The clean up in this function is non-existant. :( > > Yeah, this have a central point to do the cleanups, gb_lights_release, > since we may have other lights already configured at this point, we > could cleanup this specific one here, but than would need to make sure > all other already configure got clean also. Central clean up functions never work correctly. For example, we allocate "cdev->name" in gb_lights_channel_config() before we register the channel later in gb_lights_register_all(glights);. Now imagine that the register fails. Then when we're freeing it in __gb_lights_led_unregister() we see that the ->is_registered is false so we don't kfree(cdev->name). That's just a small memory leak. But there are going to be tons of little bugs like that. Anyway it doesn't affect this patch so it's fine. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 3/4] arm64: dts: rockchip: add rx0 mipi-phy for rk3399
Am Donnerstag, 2. April 2020, 15:48:02 CEST schrieb Johan Jonker:
> Hi Helen,
>
> > From: Helen Koike
>
> > diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> > b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> > index 33cc21fcf4c10..fc0295d2a65a1 100644
> > --- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> > +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> > @@ -1394,6 +1394,17 @@ io_domains: io-domains {
> > status = "disabled";
> > };
> >
>
> > + mipi_dphy_rx0: mipi-dphy-rx0 {
>
> For Heiko sort syscon@ff77 subnodes alphabetical or reg value first?
Similar to main nodes ... so things without reg alphabetical,
the rest by reg address
>
> > + compatible = "rockchip,rk3399-mipi-dphy-rx0";
> > + clocks = <&cru SCLK_MIPIDPHY_REF>,
>
> > + <&cru SCLK_DPHY_RX0_CFG>,
> > + <&cru PCLK_VIO_GRF>;
>
> Align^
>
> > + clock-names = "dphy-ref", "dphy-cfg", "grf";
> > + power-domains = <&power RK3399_PD_VIO>;
> > + #phy-cells = <0>;
> > + status = "disabled";
> > + };
> > +
> > u2phy0: usb2-phy@e450 {
> > compatible = "rockchip,rk3399-usb2phy";
> > reg = <0xe450 0x10>;
>
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 3/4] arm64: dts: rockchip: add rx0 mipi-phy for rk3399
On 4/2/20 4:31 PM, Heiko Stübner wrote:
> Am Donnerstag, 2. April 2020, 15:48:02 CEST schrieb Johan Jonker:
>> Hi Helen,
>>
>>> From: Helen Koike
>>
>>> diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
>>> b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
>>> index 33cc21fcf4c10..fc0295d2a65a1 100644
>>> --- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
>>> +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
>>> @@ -1394,6 +1394,17 @@ io_domains: io-domains {
>>> status = "disabled";
>>> };
>>>
>>
>>> + mipi_dphy_rx0: mipi-dphy-rx0 {
>>
>> For Heiko sort syscon@ff77 subnodes alphabetical or reg value first?
>
> Similar to main nodes ... so things without reg alphabetical,
> the rest by reg address
>
alphabetical first:
io-domains
mipi-dphy-rx0
usb2-phy@e450
.@..
or
with reg values first:
.@..
emmc_phy: phy@f780
mipi-dphy-rx0
pcie-phy
>
>>
>>> + compatible = "rockchip,rk3399-mipi-dphy-rx0";
>>> + clocks = <&cru SCLK_MIPIDPHY_REF>,
>>
>>> + <&cru SCLK_DPHY_RX0_CFG>,
>>> + <&cru PCLK_VIO_GRF>;
>>
>> Align^
>>
>>> + clock-names = "dphy-ref", "dphy-cfg", "grf";
>>> + power-domains = <&power RK3399_PD_VIO>;
>>> + #phy-cells = <0>;
>>> + status = "disabled";
>>> + };
>>> +
>>> u2phy0: usb2-phy@e450 {
>>> compatible = "rockchip,rk3399-usb2phy";
>>> reg = <0xe450 0x10>;
>>
>>
>
>
>
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 3/4] arm64: dts: rockchip: add rx0 mipi-phy for rk3399
Hi,
On 4/2/20 11:31 AM, Heiko Stübner wrote:
> Am Donnerstag, 2. April 2020, 15:48:02 CEST schrieb Johan Jonker:
>> Hi Helen,
>>
>>> From: Helen Koike
>>
>>> diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
>>> b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
>>> index 33cc21fcf4c10..fc0295d2a65a1 100644
>>> --- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
>>> +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
>>> @@ -1394,6 +1394,17 @@ io_domains: io-domains {
>>> status = "disabled";
>>> };
>>>
>>
>>> + mipi_dphy_rx0: mipi-dphy-rx0 {
>>
>> For Heiko sort syscon@ff77 subnodes alphabetical or reg value first?
>
> Similar to main nodes ... so things without reg alphabetical,
> the rest by reg address
>
>
>>
>>> + compatible = "rockchip,rk3399-mipi-dphy-rx0";
>>> + clocks = <&cru SCLK_MIPIDPHY_REF>,
>>
>>> + <&cru SCLK_DPHY_RX0_CFG>,
>>> + <&cru PCLK_VIO_GRF>;
>>
>> Align^
ack.
Thanks
Helen
>>
>>> + clock-names = "dphy-ref", "dphy-cfg", "grf";
>>> + power-domains = <&power RK3399_PD_VIO>;
>>> + #phy-cells = <0>;
>>> + status = "disabled";
>>> + };
>>> +
>>> u2phy0: usb2-phy@e450 {
>>> compatible = "rockchip,rk3399-usb2phy";
>>> reg = <0xe450 0x10>;
>>
>>
>
>
>
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/4] dt-bindings: media: rkisp1: move rockchip-isp1 bindings out of staging
Hi Johan,
Thanks for your review.
On 4/2/20 8:35 AM, Johan Jonker wrote:
> Hi Helen,
>
>> # SPDX-License-Identifier: (GPL-2.0+ OR MIT)
>> %YAML 1.2
>> ---
>> $id: http://devicetree.org/schemas/media/rockchip-isp1.yaml#
>> $schema: http://devicetree.org/meta-schemas/core.yaml#
>>
>
>> title: Rockchip SoC Image Signal Processing unit v1
>
> Where do we need 'v1' for? Is there a 'v2'?
ISPv1 is the Rockchip's name for the IP block.
>
>>
>> maintainers:
>> - Helen Koike
>>
>> description: |
>> Rockchip ISP1 is the Camera interface for the Rockchip series of SoCs
>> which contains image processing, scaling, and compression functions.
>>
>> properties:
>> compatible:
>> const: rockchip,rk3399-cif-isp
>>
>> reg:
>> maxItems: 1
>>
>> interrupts:
>> maxItems: 1
>>
>> iommus:
>> maxItems: 1
>>
>> power-domains:
>> maxItems: 1
>>
>> phys:
>> maxItems: 1
>> description: phandle for the PHY port
>>
>> phy-names:
>> const: dphy
>>
>> clocks:
>> items:
>> - description: ISP clock
>> - description: ISP AXI clock clock
>> - description: ISP AXI clock wrapper clock
>> - description: ISP AHB clock clock
>> - description: ISP AHB wrapper clock
>>
>> clock-names:
>> items:
>> - const: clk_isp
>> - const: aclk_isp
>> - const: aclk_isp_wrap
>> - const: hclk_isp
>> - const: hclk_isp_wrap
>>
>> # See ./video-interfaces.txt for details
>> ports:
>> type: object
>> additionalProperties: false
>>
>> properties:
>> "#address-cells":
>> const: 1
>>
>> "#size-cells":
>> const: 0
>>
>> port@0:
>> type: object
>> description: connection point for sensors at MIPI-DPHY RX0
>
>> additionalProperties: false
>
> Nothing required here?
I was thinking that if there is no endpoint, then nothing is required.
But if there is, then #address-cells, #size-cells and reg are. I guess
I can just add them as required.
I'll add it in the patchseries.
>
>>
>> properties:
>> "#address-cells":
>> const: 1
>>
>> "#size-cells":
>> const: 0
>>
>> reg:
>> const: 0
>>
>> patternProperties:
>> endpoint:
>> type: object
>> additionalProperties: false
>>
>> properties:
>> reg:
>> maxItems: 1
>>
>> data-lanes:
>> minItems: 1
>> maxItems: 4
>>
>> remote-endpoint: true
>>
>> required:
>
>> - port@0
>
> The use of '@0' makes "#address-cells" and "#size-cells" also a requirement.
>
> - "#address-cells"
> - "#size-cells"
Ok, I'll add it.
>
>>
>> required:
>> - compatible
>
> How about 'reg'?
>
> - reg
ack, I'll add another patch in the series fixing this.
>
>> - interrupts
>> - clocks
>> - clock-names
>> - power-domains
>> - iommus
>> - phys
>> - phy-names
>> - ports
>>
>> additionalProperties: false
>>
>> examples:
>> - |
>>
>> #include
>> #include
>> #include
>>
>> parent0: parent@0 {
>> #address-cells = <2>;
>> #size-cells = <2>;
>>
>> isp0: isp0@ff91 {
>> compatible = "rockchip,rk3399-cif-isp";
>> reg = <0x0 0xff91 0x0 0x4000>;
>> interrupts = ;
>> clocks = <&cru SCLK_ISP0>,
>> <&cru ACLK_ISP0>, <&cru ACLK_ISP0_WRAPPER>,
>> <&cru HCLK_ISP0>, <&cru HCLK_ISP0_WRAPPER>;
>> clock-names = "clk_isp",
>> "aclk_isp", "aclk_isp_wrap",
>> "hclk_isp", "hclk_isp_wrap";
>> power-domains = <&power RK3399_PD_ISP0>;
>> iommus = <&isp0_mmu>;
>> phys = <&dphy>;
>> phy-names = "dphy";
>>
>> ports {
>> #address-cells = <1>;
>> #size-cells = <0>;
>>
>> port@0 {
>> #address-cells = <1>;
>> #size-cells = <0>;
>> reg = <0>;
>>
>> mipi_in_wcam: endpoint@0 {
>> reg = <0>;
>> remote-endpoint = <&wcam_out>;
>> data-lanes = <1 2>;
>> };
>>
>> mipi_in_ucam: endpoint@1 {
>> reg = <1>;
>> remote-endpoint = <&ucam_out>;
>> data-lanes = <1>;
>> };
>> };
>> };
>> };
>>
>
>> i2c7: i2c@ff16 {
>> clock-frequency = <40>;
>> #address-cells = <1>;
>> #size-cells = <0>;
>
> Incomplete example.
> From i2c-rk3x.yaml:
>
> required:
> - compatible
> - reg
> - interrupts
> - clocks
> - clock-names
The idea was to exemplify how to connect to the sensor nodes below.
Bu
Re: [PATCH 1/4] dt-bindings: phy: phy-rockchip-dphy-rx0: move rockchip dphy rx0 bindings out of staging
Hi Johan,
On 4/2/20 9:16 AM, Johan Jonker wrote:
> Hi Helen,
>
>> # SPDX-License-Identifier: (GPL-2.0+ OR MIT)
>> %YAML 1.2
>> ---
>> $id: http://devicetree.org/schemas/phy/rockchip-mipi-dphy-rx0.yaml#
>> $schema: http://devicetree.org/meta-schemas/core.yaml#
>>
>> title: Rockchip SoC MIPI RX0 D-PHY Device Tree Bindings
>>
>> maintainers:
>> - Helen Koike
>> - Ezequiel Garcia
>>
>> description: |
>> The Rockchip SoC has a MIPI D-PHY bus with an RX0 entry which connects to
>> the ISP1 (Image Signal Processing unit v1.0) for CSI cameras.
>>
>> properties:
>> compatible:
>> const: rockchip,rk3399-mipi-dphy-rx0
>>
>
>> reg:
>> maxItems: 1
>
> If 'reg' is not used => remove it.
ok, I'll add a patch removing it.
Thanks,
Helen
>
>>
>> clocks:
>> items:
>> - description: MIPI D-PHY ref clock
>> - description: MIPI D-PHY RX0 cfg clock
>> - description: Video in/out general register file clock
>>
>> clock-names:
>> items:
>> - const: dphy-ref
>> - const: dphy-cfg
>> - const: grf
>>
>> '#phy-cells':
>> const: 0
>>
>> power-domains:
>> description: Video in/out power domain.
>> maxItems: 1
>>
>> required:
>> - compatible
>> - clocks
>> - clock-names
>> - '#phy-cells'
>> - power-domains
>>
>> additionalProperties: false
>>
>> examples:
>> - |
>>
>> /*
>> * MIPI D-PHY RX0 use registers in "general register files", it
>> * should be a child of the GRF.
>> *
>> * grf: syscon@ff77 {
>> * compatible = "rockchip,rk3399-grf", "syscon", "simple-mfd";
>> * ...
>> * };
>> */
>>
>> #include
>> #include
>>
>> mipi_dphy_rx0: mipi-dphy-rx0 {
>> compatible = "rockchip,rk3399-mipi-dphy-rx0";
>> clocks = <&cru SCLK_MIPIDPHY_REF>,
>> <&cru SCLK_DPHY_RX0_CFG>,
>> <&cru PCLK_VIO_GRF>;
>> clock-names = "dphy-ref", "dphy-cfg", "grf";
>> power-domains = <&power RK3399_PD_VIO>;
>> #phy-cells = <0>;
>> };
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 04/32] staging: wfx: remove "burst" mechanism
On Thursday 2 April 2020 15:05:26 CEST Dan Carpenter wrote:
[...]
> ^
> Not related to this this patch but this confused me initially. UINT_MAX
> would be more readable.
>
> The other unrelated question I had about this function was:
>
>402 /* search for a winner using edca params */
>403 for (i = 0; i < IEEE80211_NUM_ACS; ++i) {
> ^
> IEEE80211_NUM_ACS is 4.
>
>404 int queued;
>405
>406 edca = &wvif->edca_params[i];
>407 queued =
> wfx_tx_queue_get_num_queued(&wvif->wdev->tx_queue[i],
>408 tx_allowed_mask);
>409 if (!queued)
>410 continue;
>411 *total += queued;
>412 score = ((edca->aifs + edca->cw_min) << 16) +
>413 ((edca->cw_max - edca->cw_min) *
>414 (get_random_int() & 0x));
>415 if (score < best && (winner < 0 || i != 3)) {
>^^
>
> Why do we not want winner to be 3? It's unrelated to the patch but
> there should be a comment next to that code probably.
>
>416 best = score;
>417 winner = i;
>418 }
>419 }
Indeed. In add, this code is useless. That's why I drop this code in
patch 22/32.
--
Jérôme Pouiller
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 3/4] arm64: dts: rockchip: add rx0 mipi-phy for rk3399
Am Donnerstag, 2. April 2020, 16:37:52 CEST schrieb Johan Jonker:
> On 4/2/20 4:31 PM, Heiko Stübner wrote:
> > Am Donnerstag, 2. April 2020, 15:48:02 CEST schrieb Johan Jonker:
> >> Hi Helen,
> >>
> >>> From: Helen Koike
> >>
> >>> diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> >>> b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> >>> index 33cc21fcf4c10..fc0295d2a65a1 100644
> >>> --- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> >>> +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> >>> @@ -1394,6 +1394,17 @@ io_domains: io-domains {
> >>> status = "disabled";
> >>> };
> >>>
> >>
> >>> + mipi_dphy_rx0: mipi-dphy-rx0 {
> >>
> >> For Heiko sort syscon@ff77 subnodes alphabetical or reg value first?
> >
> > Similar to main nodes ... so things without reg alphabetical,
> > the rest by reg address
> >
> alphabetical first:
>
> io-domains
> mipi-dphy-rx0
> usb2-phy@e450
like this ... aka similar to what we do in the core nodes.
For the record, pinctrl at the bottom of a soc.dtsi is ok.
Heiko
> .@..
>
> or
>
> with reg values first:
>
> .@..
> emmc_phy: phy@f780
> mipi-dphy-rx0
> pcie-phy
>
> >
> >>
> >>> + compatible = "rockchip,rk3399-mipi-dphy-rx0";
> >>> + clocks = <&cru SCLK_MIPIDPHY_REF>,
> >>
> >>> + <&cru SCLK_DPHY_RX0_CFG>,
> >>> + <&cru PCLK_VIO_GRF>;
> >>
> >> Align^
> >>
> >>> + clock-names = "dphy-ref", "dphy-cfg", "grf";
> >>> + power-domains = <&power RK3399_PD_VIO>;
> >>> + #phy-cells = <0>;
> >>> + status = "disabled";
> >>> + };
> >>> +
> >>> u2phy0: usb2-phy@e450 {
> >>> compatible = "rockchip,rk3399-usb2phy";
> >>> reg = <0xe450 0x10>;
> >>
> >>
> >
> >
> >
> >
>
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 08/32] staging: wfx: simplify hif_handle_tx_data()
On Thursday 2 April 2020 15:13:39 CEST Dan Carpenter wrote:
> On Wed, Apr 01, 2020 at 01:03:41PM +0200, Jerome Pouiller wrote:
[...]
> This is on the TX side so it's probably okay, but one problem I have
> noticed is that we do this on the RX side as well with checking that
>
> if (skb->len < sizeof(struct hif_msg))
> return -EINVAL;
>
> So we could be reading beyond the end of the skb. If we got really
> unlucky it could lead to an Oops.
>
> regards,
> dan carpenter
>
>
Hello Dan,
The function rx_helper() in bh.c already do some sanity checks received data:
60 WARN(read_len < 4, "corrupted read");
[...]
92 } else {
93 computed_len = round_up(hif->len, 2);
94 }
95 if (computed_len != read_len) {
96 dev_err(wdev->dev, "inconsistent message length: %zu !=
%zu\n",
97 computed_len, read_len);
98 print_hex_dump(KERN_INFO, "hif: ", DUMP_PREFIX_OFFSET,
16, 1,
99 hif, read_len, true);
100 goto err;
101 }
However, I can improve this code:
- "4" should be replaced by "sizeof(struct hif_msg)" for readability
- hif->len is tested through computed_len, but I am not sure to be able
to prove that it covers all cases
- rx_helper() should recover the error if read_len < 4
I add that on my TODO list.
--
Jérôme Pouiller
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v4 9/9] arm64: dts: renesas: salvator: add a connection from adv748x codec (HDMI input) to the R-Car SoC
Hi Geert,
I'm sorry for late reply. Some unrelated happenings here in south Germany.
Geert Uytterhoeven, Mon, Mar 30, 2020 10:32:47 +0200:
> On Thu, Mar 26, 2020 at 11:55 AM Alex Riesen
> wrote:
> > --- a/arch/arm64/boot/dts/renesas/salvator-common.dtsi
> > +++ b/arch/arm64/boot/dts/renesas/salvator-common.dtsi
> > @@ -510,6 +511,15 @@ adv7482_txb: endpoint {
> > remote-endpoint = <&csi20_in>;
> > };
> > };
> > +
> > + port@c {
> > + reg = <12>;
> > +
> > + adv7482_i2s: endpoint {
> > + remote-endpoint = <&rsnd_endpoint3>;
> > + system-clock-direction-out;
> > + };
> > + };
>
> As the adv748x driver just ignores "invalid" endpoints...
>
> > @@ -758,8 +769,19 @@ &rcar_sound {
> > <&cpg CPG_MOD 1020>, <&cpg CPG_MOD 1021>,
> > <&cpg CPG_MOD 1019>, <&cpg CPG_MOD 1018>,
> > <&audio_clk_a>, <&cs2000>,
> > -<&audio_clk_c>,
> > +<&adv7482_hdmi_in>,
> > <&cpg CPG_CORE CPG_AUDIO_CLK_I>;
>
> ... and the rsnd driver ignores nonexistent-clocks, the DT change has no
> hard dependency on the driver change, and won't introduce regressions
> when included, right?
Well, it maybe won't, but isn't it a little ... implicit?
And I'm no haste to include the changes, if you mean I can (or should) submit
the device tree patch separately.
> > @@ -777,6 +799,21 @@ rsnd_endpoint0: endpoint {
> > capture = <&ssi1 &src1 &dvc1>;
> > };
> > };
> > + rsnd_port3: port@3 {
> > + reg = <3>;
> > + rsnd_endpoint3: endpoint {
> > + remote-endpoint = <&adv7482_i2s>;
> > +
> > + dai-tdm-slot-num = <8>;
> > + dai-tdm-slot-width = <32>;
> > + dai-format = "left_j";
> > + mclk-fs = <256>;
> > + bitclock-master = <&adv7482_i2s>;
> > + frame-master = <&adv7482_i2s>;
> > +
> > + capture = <&ssi4>;
> > + };
> > + };
> > };
> > };
>
> However, as salvator-common.dtsi is shared by all Salvator-X(S) variants,
> you'll have to add a dummy ssi4 node to r8a77961.dtsi first.
I see. There are even two dummy SSI nodes already. I would prefer to submit
the change together with other Salvator device tree changes. Is that alright?
Regards,
Alex
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v4 9/9] arm64: dts: renesas: salvator: add a connection from adv748x codec (HDMI input) to the R-Car SoC
Hi Alex,
On Thu, Apr 2, 2020 at 5:03 PM Alex Riesen wrote:
> Geert Uytterhoeven, Mon, Mar 30, 2020 10:32:47 +0200:
> > On Thu, Mar 26, 2020 at 11:55 AM Alex Riesen
> > wrote:
> > > --- a/arch/arm64/boot/dts/renesas/salvator-common.dtsi
> > > +++ b/arch/arm64/boot/dts/renesas/salvator-common.dtsi
> > > @@ -510,6 +511,15 @@ adv7482_txb: endpoint {
> > > remote-endpoint = <&csi20_in>;
> > > };
> > > };
> > > +
> > > + port@c {
> > > + reg = <12>;
> > > +
> > > + adv7482_i2s: endpoint {
> > > + remote-endpoint = <&rsnd_endpoint3>;
> > > + system-clock-direction-out;
> > > + };
> > > + };
> >
> > As the adv748x driver just ignores "invalid" endpoints...
> >
> > > @@ -758,8 +769,19 @@ &rcar_sound {
> > > <&cpg CPG_MOD 1020>, <&cpg CPG_MOD 1021>,
> > > <&cpg CPG_MOD 1019>, <&cpg CPG_MOD 1018>,
> > > <&audio_clk_a>, <&cs2000>,
> > > -<&audio_clk_c>,
> > > +<&adv7482_hdmi_in>,
> > > <&cpg CPG_CORE CPG_AUDIO_CLK_I>;
> >
> > ... and the rsnd driver ignores nonexistent-clocks, the DT change has no
> > hard dependency on the driver change, and won't introduce regressions
> > when included, right?
>
> Well, it maybe won't, but isn't it a little ... implicit?
> And I'm no haste to include the changes, if you mean I can (or should) submit
> the device tree patch separately.
OK, fine for me to postpone (that'll be for v5.9, I guess?).
> > > @@ -777,6 +799,21 @@ rsnd_endpoint0: endpoint {
> > > capture = <&ssi1 &src1 &dvc1>;
> > > };
> > > };
> > > + rsnd_port3: port@3 {
> > > + reg = <3>;
> > > + rsnd_endpoint3: endpoint {
> > > + remote-endpoint = <&adv7482_i2s>;
> > > +
> > > + dai-tdm-slot-num = <8>;
> > > + dai-tdm-slot-width = <32>;
> > > + dai-format = "left_j";
> > > + mclk-fs = <256>;
> > > + bitclock-master = <&adv7482_i2s>;
> > > + frame-master = <&adv7482_i2s>;
> > > +
> > > + capture = <&ssi4>;
> > > + };
> > > + };
> > > };
> > > };
> >
> > However, as salvator-common.dtsi is shared by all Salvator-X(S) variants,
> > you'll have to add a dummy ssi4 node to r8a77961.dtsi first.
>
> I see. There are even two dummy SSI nodes already. I would prefer to submit
> the change together with other Salvator device tree changes. Is that alright?
Fine for me.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v3] staging: wilc1000: Use crc7 in lib/ rather than a private copy
On Thu, Apr 02, 2020 at 11:27:45AM +0300, Dan Carpenter wrote: > I don't know how this patch made it through two versions without anyone > complaining that this paragraph should be done as a separate patch... I often fold comment (and spacing/formatting) patches in to a main patch, when touching adjacent code anyway and it doesn't cause distracting clutter. This seemed like such a case, which is why I submitted it as one. But it's a bit of style thing. >> Cc: Adham Abozaeid >> Cc: linux-wirel...@vger.kernel.org >> Reviewed-by: Ajay Singh >> Signed-off-by: George Spelvin >> --- > > This should have you Signed-off-by. The Reviewed-by is kind of assumed > so you can drop that bit. But everyone who touches a patch needs to > add their signed off by. Er... all he did was add "staging: " to the front of the title. That's not a change to the code at all, and as trivial a change to the commit message as adding "Reviewed-by:" to the end. We don't need S-o-b for such things or we'd end up in a horrible infinite recursion. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v4 9/9] arm64: dts: renesas: salvator: add a connection from adv748x codec (HDMI input) to the R-Car SoC
Geert Uytterhoeven, Thu, Apr 02, 2020 17:26:15 +0200:
> On Thu, Apr 2, 2020 at 5:03 PM Alex Riesen
> wrote:
> > Geert Uytterhoeven, Mon, Mar 30, 2020 10:32:47 +0200:
> > > On Thu, Mar 26, 2020 at 11:55 AM Alex Riesen
> > > wrote:
> > > > --- a/arch/arm64/boot/dts/renesas/salvator-common.dtsi
> > > > +++ b/arch/arm64/boot/dts/renesas/salvator-common.dtsi
> > > > @@ -510,6 +511,15 @@ adv7482_txb: endpoint {
> > > > remote-endpoint = <&csi20_in>;
> > > > };
> > > > };
> > > > +
> > > > + port@c {
> > > > + reg = <12>;
> > > > +
> > > > + adv7482_i2s: endpoint {
> > > > + remote-endpoint = <&rsnd_endpoint3>;
> > > > + system-clock-direction-out;
> > > > + };
> > > > + };
> > >
> > > As the adv748x driver just ignores "invalid" endpoints...
> > >
> > > > @@ -758,8 +769,19 @@ &rcar_sound {
> > > > <&cpg CPG_MOD 1020>, <&cpg CPG_MOD 1021>,
> > > > <&cpg CPG_MOD 1019>, <&cpg CPG_MOD 1018>,
> > > > <&audio_clk_a>, <&cs2000>,
> > > > -<&audio_clk_c>,
> > > > +<&adv7482_hdmi_in>,
> > > > <&cpg CPG_CORE CPG_AUDIO_CLK_I>;
> > >
> > > ... and the rsnd driver ignores nonexistent-clocks, the DT change has no
> > > hard dependency on the driver change, and won't introduce regressions
> > > when included, right?
> >
> > Well, it maybe won't, but isn't it a little ... implicit?
> > And I'm no haste to include the changes, if you mean I can (or should)
> > submit
> > the device tree patch separately.
>
> OK, fine for me to postpone (that'll be for v5.9, I guess?).
>
Looks scary :)
But yes, fine with me too.
v5 with ssi4 dummy in a moment.
Regards,
Alex
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH -next] staging: greybus: fix a missing-check bug in gb_lights_light_config()
Hi, On Thu, Apr 02, 2020 at 05:22:37PM +0300, Dan Carpenter wrote: > On Thu, Apr 02, 2020 at 02:16:18PM +0100, Rui Miguel Silva wrote: > > > > --- a/drivers/staging/greybus/light.c > > > > +++ b/drivers/staging/greybus/light.c > > > > @@ -1026,7 +1026,8 @@ static int gb_lights_light_config(struct > > > > gb_lights *glights, u8 id) > > > > > > > > light->channels_count = conf.channel_count; > > > > light->name = kstrndup(conf.name, NAMES_MAX, GFP_KERNEL); > > > > - > > > > + if (!light->name) > > > > + return -ENOMEM; > > > > light->channels = kcalloc(light->channels_count, > > > > sizeof(struct gb_channel), > > > > GFP_KERNEL); > > > > if (!light->channels) > > > > > > The clean up in this function is non-existant. :( > > > > Yeah, this have a central point to do the cleanups, gb_lights_release, > > since we may have other lights already configured at this point, we > > could cleanup this specific one here, but than would need to make sure > > all other already configure got clean also. > > Central clean up functions never work correctly. I agree. > > For example, we allocate "cdev->name" in gb_lights_channel_config() > before we register the channel later in gb_lights_register_all(glights);. > Now imagine that the register fails. Then when we're freeing it in > __gb_lights_led_unregister() we see that the ->is_registered is false > so we don't kfree(cdev->name). > > That's just a small memory leak. But there are going to be tons of > little bugs like that. Yeah, when I have some cycles I'll go over that error codes paths and mitigate this kind of issues. > > Anyway it doesn't affect this patch so it's fine. Yeah, thanks. -- Cheers, Rui ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 01/32] staging: wfx: add sanity checks to hif_join()
On Thursday 2 April 2020 14:42:23 CEST Dan Carpenter wrote:
> On Wed, Apr 01, 2020 at 01:03:34PM +0200, Jerome Pouiller wrote:
> > From: Jérôme Pouiller
> >
> > Add a few check on start of hif_join().
> >
> > Signed-off-by: Jérôme Pouiller
> > ---
> > drivers/staging/wfx/hif_tx.c | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/staging/wfx/hif_tx.c b/drivers/staging/wfx/hif_tx.c
> > index 77bca43aca42..445906035e9d 100644
> > --- a/drivers/staging/wfx/hif_tx.c
> > +++ b/drivers/staging/wfx/hif_tx.c
> > @@ -297,6 +297,8 @@ int hif_join(struct wfx_vif *wvif, const struct
> > ieee80211_bss_conf *conf,
> > struct hif_req_join *body = wfx_alloc_hif(sizeof(*body), &hif);
>
> We've got an allocation here. It's a mistake to put the allocation in
> the declaration block because you're going to forget to check for
> failure.
arf... this remark also applies to all functions of hif_tx.c. This
issue has already been reported. I will send a patch that solve that in
one batch.
> > WARN_ON(!conf->basic_rates);
> > + WARN_ON(sizeof(body->ssid) < ssidlen);
>
> Put the variable on the left. WARN_ON(ssidlen > sizeof(body->ssid)).
> I'm not a big fan of adding this sort of debug code, just audit the
> callers to see if it's possible or not.
My personal opinion is these checks does not replace the audit of the
callers. It mainly provides a kind of documentation for the reader
("not supported, please check the callers"). It is especially true when
it is an internal API and there is only one caller.
> I have audited the caller for you, and I believe that this condition
> *is possible* so we need to return -EINVAL in this situation to prevent
> memory corruption.
>
> if (ssidlen > sizeof(body->ssid))
> return -EINVAL;
In this case, I think the problem will also impact wfx_do_join() (the
only caller of hif_join()):
514 u8 ssid[IEEE80211_MAX_SSID_LEN];
[...]
538 if (!conf->ibss_joined)
539 ssidie = ieee80211_bss_get_ie(bss, WLAN_EID_SSID);
540 if (ssidie) {
541 ssidlen = ssidie[1];
542 memcpy(ssid, &ssidie[2], ssidie[1]);
543 }
[...]
554 ret = hif_join(wvif, conf, wvif->channel, ssid, ssidlen);
Does data returned by ieee80211_bss_get_ie() could be bigger than
IEEE80211_MAX_SSID_LEN? Not sure. I am going to add a check in
wfx_do_join(), just in case.
--
Jérôme Pouiller
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/4] dt-bindings: phy: phy-rockchip-dphy-rx0: move rockchip dphy rx0 bindings out of staging
(+Kishon)
Hi Helen,
I was wondering if we couldn't also move the phy driver out of staging.
Thanks,
Ezequiel
On Wed, 2020-04-01 at 21:02 -0300, Helen Koike wrote:
> Move phy-rockchip-dphy-rx0 bindings to Documentation/devicetree/bindings/phy
>
> Signed-off-by: Helen Koike
> ---
> .../devicetree/bindings/phy/rockchip-mipi-dphy-rx0.yaml | 0
> 1 file changed, 0 insertions(+), 0 deletions(-)
> rename {drivers/staging/media/phy-rockchip-dphy-rx0/Documentation =>
> Documentation}/devicetree/bindings/phy/rockchip-mipi-dphy-rx0.yaml (100%)
>
> diff --git
> a/drivers/staging/media/phy-rockchip-dphy-rx0/Documentation/devicetree/bindings/phy/rockchip-mipi-dphy-rx0.yaml
> b/Documentation/devicetree/bindings/phy/rockchip-mipi-dphy-rx0.yaml
> similarity index 100%
> rename from
> drivers/staging/media/phy-rockchip-dphy-rx0/Documentation/devicetree/bindings/phy/rockchip-mipi-dphy-rx0.yaml
> rename to Documentation/devicetree/bindings/phy/rockchip-mipi-dphy-rx0.yaml
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: vt6656: Use defines in vnt_mac_reg_bits_* functions
On Thu, Apr 02, 2020 at 11:58:07AM +0100, Malcolm Priestley wrote:
>
>
> On 02/04/2020 10:19, Quentin Deslandes wrote:
> > On 04/01/20 18:55:38, Oscar Carter wrote:
> > > On Tue, Mar 31, 2020 at 01:29:06PM +0300, Dan Carpenter wrote:
> > > > On Sat, Mar 28, 2020 at 10:54:33AM +0100, Oscar Carter wrote:
> > > > > Define the necessary bits in the CHANNEL, PAPEDELAY and GPIOCTL0
> > > > > registers to can use them in the calls to vnt_mac_reg_bits_on and
> > > > > vnt_mac_reg_bits_off functions. In this way, avoid the use of BIT()
> > > > > macros and clarify the code.
> > > > >
> > > > > Fixes: 3017e587e368 ("staging: vt6656: Use BIT() macro in
> > > > > vnt_mac_reg_bits_* functions")
> > > > > Suggested-by: Dan Carpenter
> > > > > Signed-off-by: Oscar Carter
> > > > > ---
> > > > > drivers/staging/vt6656/baseband.c | 6 --
> > > > > drivers/staging/vt6656/card.c | 3 +--
> > > > > drivers/staging/vt6656/mac.h | 12
> > > > > drivers/staging/vt6656/main_usb.c | 2 +-
> > > > > 4 files changed, 18 insertions(+), 5 deletions(-)
> > > > >
> > > > > diff --git a/drivers/staging/vt6656/baseband.c
> > > > > b/drivers/staging/vt6656/baseband.c
> > > > > index a19a563d8bcc..dd3c3bf5e8b5 100644
> > > > > --- a/drivers/staging/vt6656/baseband.c
> > > > > +++ b/drivers/staging/vt6656/baseband.c
> > > > > @@ -442,7 +442,8 @@ int vnt_vt3184_init(struct vnt_private *priv)
> > > > > if (ret)
> > > > > goto end;
> > > > >
> > > > > - ret = vnt_mac_reg_bits_on(priv, MAC_REG_PAPEDELAY,
> > > > > BIT(0));
> > > > > + ret = vnt_mac_reg_bits_on(priv, MAC_REG_PAPEDELAY,
> > > > > + PAPEDELAY_B0);
> > > >
> > > > This doesn't clarify anything. It makes it less clear because someone
> > > > would assume B0 means something but it's just hiding a magic number
> > > > behind a meaningless define. B0 means BIT(0) which means nothing. So
> > > > now we have to jump through two hoops to find out that we don't know
> > > > anything.
> > > >
> > > I created this names due to the lack of information about the hardware. I
> > > searched but I did not find anything.
> > >
> > > > Just leave it as-is. Same for the rest.
> > > Ok.
> > >
> > > >
> > > > There problem is a hardware spec which explains what this stuff is.
> > > >
> > > It's possible to find a datasheet of this hardware to make this
> > > modification
> > > accordingly to the correct bit names of registers ?
> >
> > I haven't found any so far, if your researches are luckier than mine, I
> > would be interested too. Even getting your hands on the actual device is
> > complicated.
>
> All I can tell you is it related to command above it MAC_REG_ITRTMSET
> without it the device will not associate with access point is probably TX
> timing/power rated.
>
> Other bits in MAC_REG_PAPEDELAY are related to LED function and defined in
> LEDSTS_* in mac.h.
>
> I think it is some kind of enable so something like ITRTMSET_ENABLE would
> do.
>
I think the best for now is leave it as-is due to the lack of information about
bit names of registers.
> Regards
>
> Malcolm
Thanks,
Oscar carter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2] staging: vt6656: Define EnCFG_BBType_MASK as OR between previous defines
Define the EnCFG_BBType_MASK bit as an OR operation between two previous defines instead of using the OR between two new BIT macros. Thus, the code is more clear. Signed-off-by: Oscar Carter Reviewed-by: Dan Carpenter Reviewed-by: Quentin Deslandes --- Changelog v1 -> v2 - Remove the "Fixes:" tag line. - Add "Reviewed-by: Quentin Deslandes" drivers/staging/vt6656/mac.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/vt6656/mac.h b/drivers/staging/vt6656/mac.h index c532b27de37f..b01d9ee8677e 100644 --- a/drivers/staging/vt6656/mac.h +++ b/drivers/staging/vt6656/mac.h @@ -177,7 +177,7 @@ #define EnCFG_BBType_a 0x00 #define EnCFG_BBType_b BIT(0) #define EnCFG_BBType_g BIT(1) -#define EnCFG_BBType_MASK (BIT(0) | BIT(1)) +#define EnCFG_BBType_MASK (EnCFG_BBType_b | EnCFG_BBType_g) #define EnCFG_ProtectMdBIT(5) /* Bits in the EnhanceCFG_1 register */ -- 2.20.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 4/4] arm64: dts: rockchip: add isp0 node for rk3399
Hi Helen,
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> index fc0295d2a65a1..815099a0cd0dd 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> @@ -1718,6 +1718,33 @@ vopb_mmu: iommu@ff903f00 {
> status = "disabled";
> };
>
> + isp0: isp0@ff91 {
> + compatible = "rockchip,rk3399-cif-isp";
> + reg = <0x0 0xff91 0x0 0x4000>;
> + interrupts = ;
> + clocks = <&cru SCLK_ISP0>,
> + <&cru ACLK_ISP0>, <&cru ACLK_ISP0_WRAPPER>,
> + <&cru HCLK_ISP0>, <&cru HCLK_ISP0_WRAPPER>;
> + clock-names = "clk_isp",
> + "aclk_isp", "aclk_isp_wrap",
> + "hclk_isp", "hclk_isp_wrap";
> + power-domains = <&power RK3399_PD_ISP0>;
> + iommus = <&isp0_mmu>;
> + phys = <&mipi_dphy_rx0>;
> + phy-names = "dphy";
Maybe a little sort? But keep rest as it is. Also in example.
iommus = <&isp0_mmu>;
phys = <&mipi_dphy_rx0>;
phy-names = "dphy";
power-domains = <&power RK3399_PD_ISP0>;
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port@0 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + reg = <0>;
Move reg above #address-cells. Change that in example as well.
reg = <0>;
#address-cells = <1>;
#size-cells = <0>;
> + };
> + };
> + };
> +
> isp0_mmu: iommu@ff914000 {
> compatible = "rockchip,iommu";
> reg = <0x0 0xff914000 0x0 0x100>, <0x0 0xff915000 0x0 0x100>;
> --
> 2.26.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v5 0/9] media: adv748x: add support for HDMI audio
This adds minimal support for accessing the HDMI audio provided through the I2S port available on ADV7481 and ADV7482 decoder devices by ADI. The port carries audio signal from the decoded HDMI stream. Currently, the driver only supports I2S in TDM, 8 channels a 24bit at 48kHz. Furthermore, only left-justified, 8 slots, 32bit/slot TDM, at 256fs has been ever tried. An ADV7482 on the Renesas Salvator-X ES1.1 (R8A77950 SoC) was used during development of this code. Changes since v4: - rebased on v5.6 - Add dummy ssi4 node to the rcar sound card, as the r8a77961 devices also reference salvator-common.dts. Suggested-by: Geert Uytterhoeven Changes since v3: - use clk_hw instead of clk Suggested-by: Stephen Boyd - formatting improvements and use const where possible - removed implementation of log_status and EDID setting ioctls, those will be submitted as separate patches. Suggested-by: Hans Verkuil Changes since v2: - prepare/enable the clock when it is used, as it seems nothing else does this otherwise - give the clock a unique name to ensure it can be registered if there are multiple adv748x devices in the system - remove optionality note from clock cell description to ensure the device description matches the real device (the line is always present, even if not used) Changes since v1: - Add ssi4_ctrl pin group to the sound pins. The pins are responsible for SCK4 (sample clock) WS4 and (word boundary input), and are required for SSI audio input over I2S. Reported-by: Geert Uytterhoeven - Removed the audio clock C from the list of clocks of adv748x, it is exactly the other way around. Reported-by: Geert Uytterhoeven - Add an instance of (currently) fixed rate I2S master clock (MCLK), connected to the audio_clk_c line of the R-Car SoC. Explicitly declare the device a clock producer and add it to the list of clocks used by the audio system of the Salvator-X board. Suggested-by: Geert Uytterhoeven - The implementation of DAI driver has been moved in a separate file and modified to activate audio decoding and I2S streaming using snd_soc_dai_... interfaces. This allows the driver to be used with just ALSA interfaces. - The ioctls for selecting audio output and muting have been removed, as not applicable. Suggested-by: Hans Verkuil I have left implementation of the QUERYCAP in, as it seems to be required by v4l-ctl to support loading of EDID for this node. And setting the EDID is one feature I desperately need: there are devices which plainly refuse to talk to the sink if it does not provide EDID they like. - A device tree configuration without audio port will disable the audio code altogether, supporting integrations where the port is not connected. - The patches have been re-arranged, starting with the generic changes and changes not related to audio directly. Those will be probably sent as a separate series later. - The whole series has been rebased on top of v5.6-rc6 Alex Riesen (9): media: adv748x: fix end-of-line terminators in diagnostic statements media: adv748x: include everything adv748x.h needs into the file media: adv748x: reduce amount of code for bitwise modifications of device registers media: adv748x: add definitions for audio output related registers media: adv748x: add support for HDMI audio media: adv748x: prepare/enable mclk when the audio is used media: adv748x: only activate DAI if it is described in device tree dt-bindings: adv748x: add information about serial audio interface (I2S/TDM) arm64: dts: renesas: salvator: add a connection from adv748x codec (HDMI input) to the R-Car SoC .../devicetree/bindings/media/i2c/adv748x.txt | 16 +- .../boot/dts/renesas/r8a77950-salvator-x.dts | 3 +- arch/arm64/boot/dts/renesas/r8a77961.dtsi | 1 + .../boot/dts/renesas/salvator-common.dtsi | 47 ++- drivers/media/i2c/adv748x/Makefile| 3 +- drivers/media/i2c/adv748x/adv748x-afe.c | 6 +- drivers/media/i2c/adv748x/adv748x-core.c | 45 +-- drivers/media/i2c/adv748x/adv748x-csi2.c | 8 +- drivers/media/i2c/adv748x/adv748x-dai.c | 278 ++ drivers/media/i2c/adv748x/adv748x-hdmi.c | 6 +- drivers/media/i2c/adv748x/adv748x.h | 65 +++- 11 files changed, 436 insertions(+), 42 deletions(-) create mode 100644 drivers/media/i2c/adv748x/adv748x-dai.c -- 2.25.1.25.g9ecbe7eb18 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v5 1/9] media: adv748x: fix end-of-line terminators in diagnostic statements
Signed-off-by: Alexander Riesen
Reviewed-by: Laurent Pinchart
---
drivers/media/i2c/adv748x/adv748x-core.c | 24
drivers/media/i2c/adv748x/adv748x-csi2.c | 2 +-
2 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/drivers/media/i2c/adv748x/adv748x-core.c
b/drivers/media/i2c/adv748x/adv748x-core.c
index 23e02ff27b17..c3fb113cef62 100644
--- a/drivers/media/i2c/adv748x/adv748x-core.c
+++ b/drivers/media/i2c/adv748x/adv748x-core.c
@@ -623,11 +623,11 @@ static int adv748x_parse_dt(struct adv748x_state *state)
for_each_endpoint_of_node(state->dev->of_node, ep_np) {
of_graph_parse_endpoint(ep_np, &ep);
- adv_info(state, "Endpoint %pOF on port %d", ep.local_node,
+ adv_info(state, "Endpoint %pOF on port %d\n", ep.local_node,
ep.port);
if (ep.port >= ADV748X_PORT_MAX) {
- adv_err(state, "Invalid endpoint %pOF on port %d",
+ adv_err(state, "Invalid endpoint %pOF on port %d\n",
ep.local_node, ep.port);
continue;
@@ -635,7 +635,7 @@ static int adv748x_parse_dt(struct adv748x_state *state)
if (state->endpoints[ep.port]) {
adv_err(state,
- "Multiple port endpoints are not supported");
+ "Multiple port endpoints are not supported\n");
continue;
}
@@ -702,62 +702,62 @@ static int adv748x_probe(struct i2c_client *client)
/* Discover and process ports declared by the Device tree endpoints */
ret = adv748x_parse_dt(state);
if (ret) {
- adv_err(state, "Failed to parse device tree");
+ adv_err(state, "Failed to parse device tree\n");
goto err_free_mutex;
}
/* Configure IO Regmap region */
ret = adv748x_configure_regmap(state, ADV748X_PAGE_IO);
if (ret) {
- adv_err(state, "Error configuring IO regmap region");
+ adv_err(state, "Error configuring IO regmap region\n");
goto err_cleanup_dt;
}
ret = adv748x_identify_chip(state);
if (ret) {
- adv_err(state, "Failed to identify chip");
+ adv_err(state, "Failed to identify chip\n");
goto err_cleanup_dt;
}
/* Configure remaining pages as I2C clients with regmap access */
ret = adv748x_initialise_clients(state);
if (ret) {
- adv_err(state, "Failed to setup client regmap pages");
+ adv_err(state, "Failed to setup client regmap pages\n");
goto err_cleanup_clients;
}
/* SW reset ADV748X to its default values */
ret = adv748x_reset(state);
if (ret) {
- adv_err(state, "Failed to reset hardware");
+ adv_err(state, "Failed to reset hardware\n");
goto err_cleanup_clients;
}
/* Initialise HDMI */
ret = adv748x_hdmi_init(&state->hdmi);
if (ret) {
- adv_err(state, "Failed to probe HDMI");
+ adv_err(state, "Failed to probe HDMI\n");
goto err_cleanup_clients;
}
/* Initialise AFE */
ret = adv748x_afe_init(&state->afe);
if (ret) {
- adv_err(state, "Failed to probe AFE");
+ adv_err(state, "Failed to probe AFE\n");
goto err_cleanup_hdmi;
}
/* Initialise TXA */
ret = adv748x_csi2_init(state, &state->txa);
if (ret) {
- adv_err(state, "Failed to probe TXA");
+ adv_err(state, "Failed to probe TXA\n");
goto err_cleanup_afe;
}
/* Initialise TXB */
ret = adv748x_csi2_init(state, &state->txb);
if (ret) {
- adv_err(state, "Failed to probe TXB");
+ adv_err(state, "Failed to probe TXB\n");
goto err_cleanup_txa;
}
diff --git a/drivers/media/i2c/adv748x/adv748x-csi2.c
b/drivers/media/i2c/adv748x/adv748x-csi2.c
index 2091cda50935..c43ce5d78723 100644
--- a/drivers/media/i2c/adv748x/adv748x-csi2.c
+++ b/drivers/media/i2c/adv748x/adv748x-csi2.c
@@ -72,7 +72,7 @@ static int adv748x_csi2_registered(struct v4l2_subdev *sd)
struct adv748x_state *state = tx->state;
int ret;
- adv_dbg(state, "Registered %s (%s)", is_txa(tx) ? "TXA":"TXB",
+ adv_dbg(state, "Registered %s (%s)\n", is_txa(tx) ? "TXA":"TXB",
sd->name);
/*
--
2.25.1.25.g9ecbe7eb18
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v5 2/9] media: adv748x: include everything adv748x.h needs into the file
To follow the established practice of not depending on others to
pull everything in. While at it, make sure it stays like this.
Signed-off-by: Alexander Riesen
Reviewed-by: Laurent Pinchart
---
drivers/media/i2c/adv748x/adv748x-afe.c | 6 ++
drivers/media/i2c/adv748x/adv748x-core.c | 6 ++
drivers/media/i2c/adv748x/adv748x-csi2.c | 6 ++
drivers/media/i2c/adv748x/adv748x-hdmi.c | 6 ++
drivers/media/i2c/adv748x/adv748x.h | 2 ++
5 files changed, 10 insertions(+), 16 deletions(-)
diff --git a/drivers/media/i2c/adv748x/adv748x-afe.c
b/drivers/media/i2c/adv748x/adv748x-afe.c
index dbbb1e4d6363..5a25d1fbe25f 100644
--- a/drivers/media/i2c/adv748x/adv748x-afe.c
+++ b/drivers/media/i2c/adv748x/adv748x-afe.c
@@ -6,18 +6,16 @@
* Copyright (C) 2017 Renesas Electronics Corp.
*/
+#include "adv748x.h"
+
#include
#include
#include
#include
-#include
-#include
#include
#include
-#include "adv748x.h"
-
/*
-
* SDP
*/
diff --git a/drivers/media/i2c/adv748x/adv748x-core.c
b/drivers/media/i2c/adv748x/adv748x-core.c
index c3fb113cef62..5c59aad319d1 100644
--- a/drivers/media/i2c/adv748x/adv748x-core.c
+++ b/drivers/media/i2c/adv748x/adv748x-core.c
@@ -10,6 +10,8 @@
* Kieran Bingham
*/
+#include "adv748x.h"
+
#include
#include
#include
@@ -20,14 +22,10 @@
#include
#include
-#include
-#include
#include
#include
#include
-#include "adv748x.h"
-
/*
-
* Register manipulation
*/
diff --git a/drivers/media/i2c/adv748x/adv748x-csi2.c
b/drivers/media/i2c/adv748x/adv748x-csi2.c
index c43ce5d78723..c00d4f347d95 100644
--- a/drivers/media/i2c/adv748x/adv748x-csi2.c
+++ b/drivers/media/i2c/adv748x/adv748x-csi2.c
@@ -5,15 +5,13 @@
* Copyright (C) 2017 Renesas Electronics Corp.
*/
+#include "adv748x.h"
+
#include
#include
-#include
-#include
#include
-#include "adv748x.h"
-
static int adv748x_csi2_set_virtual_channel(struct adv748x_csi2 *tx,
unsigned int vc)
{
diff --git a/drivers/media/i2c/adv748x/adv748x-hdmi.c
b/drivers/media/i2c/adv748x/adv748x-hdmi.c
index c557f8fdf11a..f598acec3b5c 100644
--- a/drivers/media/i2c/adv748x/adv748x-hdmi.c
+++ b/drivers/media/i2c/adv748x/adv748x-hdmi.c
@@ -5,18 +5,16 @@
* Copyright (C) 2017 Renesas Electronics Corp.
*/
+#include "adv748x.h"
+
#include
#include
-#include
-#include
#include
#include
#include
-#include "adv748x.h"
-
/*
-
* HDMI and CP
*/
diff --git a/drivers/media/i2c/adv748x/adv748x.h
b/drivers/media/i2c/adv748x/adv748x.h
index fccb388ce179..09aab4138c3f 100644
--- a/drivers/media/i2c/adv748x/adv748x.h
+++ b/drivers/media/i2c/adv748x/adv748x.h
@@ -19,6 +19,8 @@
*/
#include
+#include
+#include
#ifndef _ADV748X_H_
#define _ADV748X_H_
--
2.25.1.25.g9ecbe7eb18
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v5 3/9] media: adv748x: reduce amount of code for bitwise modifications of device registers
The regmap provides a convenient utility for this.
The hdmi_* and dpll_* register modification macros added for symmetry
with the existing operations (io_*, sdp_*).
Signed-off-by: Alexander Riesen
Reviewed-by: Laurent Pinchart
--
v3: remove _update name in favor of existing _clrset
---
drivers/media/i2c/adv748x/adv748x-core.c | 6 ++
drivers/media/i2c/adv748x/adv748x.h | 14 +++---
2 files changed, 17 insertions(+), 3 deletions(-)
diff --git a/drivers/media/i2c/adv748x/adv748x-core.c
b/drivers/media/i2c/adv748x/adv748x-core.c
index 5c59aad319d1..8580e6624276 100644
--- a/drivers/media/i2c/adv748x/adv748x-core.c
+++ b/drivers/media/i2c/adv748x/adv748x-core.c
@@ -133,6 +133,12 @@ static int adv748x_write_check(struct adv748x_state
*state, u8 page, u8 reg,
return *error;
}
+int adv748x_update_bits(struct adv748x_state *state, u8 page, u8 reg, u8 mask,
+ u8 value)
+{
+ return regmap_update_bits(state->regmap[page], reg, mask, value);
+}
+
/* adv748x_write_block(): Write raw data with a maximum of I2C_SMBUS_BLOCK_MAX
* size to one or more registers.
*
diff --git a/drivers/media/i2c/adv748x/adv748x.h
b/drivers/media/i2c/adv748x/adv748x.h
index 09aab4138c3f..0a9d78c2870b 100644
--- a/drivers/media/i2c/adv748x/adv748x.h
+++ b/drivers/media/i2c/adv748x/adv748x.h
@@ -393,25 +393,33 @@ int adv748x_write(struct adv748x_state *state, u8 page,
u8 reg, u8 value);
int adv748x_write_block(struct adv748x_state *state, int client_page,
unsigned int init_reg, const void *val,
size_t val_len);
+int adv748x_update_bits(struct adv748x_state *state, u8 page, u8 reg,
+ u8 mask, u8 value);
#define io_read(s, r) adv748x_read(s, ADV748X_PAGE_IO, r)
#define io_write(s, r, v) adv748x_write(s, ADV748X_PAGE_IO, r, v)
-#define io_clrset(s, r, m, v) io_write(s, r, (io_read(s, r) & ~(m)) | (v))
+#define io_clrset(s, r, m, v) adv748x_update_bits(s, ADV748X_PAGE_IO, r, m, v)
#define hdmi_read(s, r) adv748x_read(s, ADV748X_PAGE_HDMI, r)
#define hdmi_read16(s, r, m) (((hdmi_read(s, r) << 8) | hdmi_read(s, (r)+1)) &
(m))
#define hdmi_write(s, r, v) adv748x_write(s, ADV748X_PAGE_HDMI, r, v)
+#define hdmi_clrset(s, r, m, v) \
+ adv748x_update_bits(s, ADV748X_PAGE_HDMI, r, m, v)
+
+#define dpll_read(s, r) adv748x_read(s, ADV748X_PAGE_DPLL, r)
+#define dpll_clrset(s, r, m, v) \
+ adv748x_update_bits(s, ADV748X_PAGE_DPLL, r, m, v)
#define repeater_read(s, r) adv748x_read(s, ADV748X_PAGE_REPEATER, r)
#define repeater_write(s, r, v) adv748x_write(s, ADV748X_PAGE_REPEATER, r, v)
#define sdp_read(s, r) adv748x_read(s, ADV748X_PAGE_SDP, r)
#define sdp_write(s, r, v) adv748x_write(s, ADV748X_PAGE_SDP, r, v)
-#define sdp_clrset(s, r, m, v) sdp_write(s, r, (sdp_read(s, r) & ~(m)) | (v))
+#define sdp_clrset(s, r, m, v) adv748x_update_bits(s, ADV748X_PAGE_SDP, r, m,
v)
#define cp_read(s, r) adv748x_read(s, ADV748X_PAGE_CP, r)
#define cp_write(s, r, v) adv748x_write(s, ADV748X_PAGE_CP, r, v)
-#define cp_clrset(s, r, m, v) cp_write(s, r, (cp_read(s, r) & ~(m)) | (v))
+#define cp_clrset(s, r, m, v) adv748x_update_bits(s, ADV748X_PAGE_CP, r, m, v)
#define tx_read(t, r) adv748x_read(t->state, t->page, r)
#define tx_write(t, r, v) adv748x_write(t->state, t->page, r, v)
--
2.25.1.25.g9ecbe7eb18
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v5 4/9] media: adv748x: add definitions for audio output related registers
Signed-off-by: Alexander Riesen
---
drivers/media/i2c/adv748x/adv748x.h | 32 +
1 file changed, 32 insertions(+)
diff --git a/drivers/media/i2c/adv748x/adv748x.h
b/drivers/media/i2c/adv748x/adv748x.h
index 0a9d78c2870b..1a1ea70086c6 100644
--- a/drivers/media/i2c/adv748x/adv748x.h
+++ b/drivers/media/i2c/adv748x/adv748x.h
@@ -226,6 +226,11 @@ struct adv748x_state {
#define ADV748X_IO_VID_STD 0x05
+#define ADV748X_IO_PAD_CONTROLS0x0e
+#define ADV748X_IO_PAD_CONTROLS_TRI_AUDBIT(5)
+#define ADV748X_IO_PAD_CONTROLS_PDN_AUDBIT(1)
+#define ADV748X_IO_PAD_CONTROLS1 0x1d
+
#define ADV748X_IO_10 0x10/* io_reg_10 */
#define ADV748X_IO_10_CSI4_EN BIT(7)
#define ADV748X_IO_10_CSI1_EN BIT(6)
@@ -248,7 +253,21 @@ struct adv748x_state {
#define ADV748X_IO_REG_FF 0xff
#define ADV748X_IO_REG_FF_MAIN_RESET 0xff
+/* DPLL Map */
+#define ADV748X_DPLL_MCLK_FS 0xb5
+#define ADV748X_DPLL_MCLK_FS_N_MASKGENMASK(2, 0)
+
/* HDMI RX Map */
+#define ADV748X_HDMI_I2S 0x03/* I2S mode and width */
+#define ADV748X_HDMI_I2SBITWIDTH_MASK GENMASK(4, 0)
+#define ADV748X_HDMI_I2SOUTMODE_SHIFT 5
+#define ADV748X_HDMI_I2SOUTMODE_MASK \
+ GENMASK(6, ADV748X_HDMI_I2SOUTMODE_SHIFT)
+#define ADV748X_I2SOUTMODE_I2S 0
+#define ADV748X_I2SOUTMODE_RIGHT_J 1
+#define ADV748X_I2SOUTMODE_LEFT_J 2
+#define ADV748X_I2SOUTMODE_SPDIF 3
+
#define ADV748X_HDMI_LW1 0x07/* line width_1 */
#define ADV748X_HDMI_LW1_VERT_FILTER BIT(7)
#define ADV748X_HDMI_LW1_DE_REGEN BIT(5)
@@ -260,6 +279,16 @@ struct adv748x_state {
#define ADV748X_HDMI_F1H1 0x0b/* field1 height_1 */
#define ADV748X_HDMI_F1H1_INTERLACED BIT(5)
+#define ADV748X_HDMI_MUTE_CTRL 0x1a
+#define ADV748X_HDMI_MUTE_CTRL_MUTE_AUDIO BIT(4)
+#define ADV748X_HDMI_MUTE_CTRL_WAIT_UNMUTE_MASKGENMASK(3, 1)
+#define ADV748X_HDMI_MUTE_CTRL_NOT_AUTO_UNMUTE BIT(0)
+
+#define ADV748X_HDMI_AUDIO_MUTE_SPEED 0x0f
+#define ADV748X_HDMI_AUDIO_MUTE_SPEED_MASK GENMASK(4, 0)
+#define ADV748X_MAN_AUDIO_DL_BYPASS BIT(7)
+#define ADV748X_AUDIO_DELAY_LINE_BYPASS BIT(6)
+
#define ADV748X_HDMI_HFRONT_PORCH 0x20/* hsync_front_porch_1 */
#define ADV748X_HDMI_HFRONT_PORCH_MASK 0x1fff
@@ -281,6 +310,9 @@ struct adv748x_state {
#define ADV748X_HDMI_TMDS_10x51/* hdmi_reg_51 */
#define ADV748X_HDMI_TMDS_20x52/* hdmi_reg_52 */
+#define ADV748X_HDMI_REG_6D0x6d/* hdmi_reg_6d */
+#define ADV748X_I2S_TDM_MODE_ENABLE BIT(7)
+
/* HDMI RX Repeater Map */
#define ADV748X_REPEATER_EDID_SZ 0x70/* primary_edid_size */
#define ADV748X_REPEATER_EDID_SZ_SHIFT 4
--
2.25.1.25.g9ecbe7eb18
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v5 5/9] media: adv748x: add support for HDMI audio
This adds an implemention of SoC DAI driver which provides access to the
I2S port of the device.
Signed-off-by: Alexander Riesen
--
v3: fix clock registration in case of multiple adv748x devices
Suggested-by: Geert Uytterhoeven
v4: use clk_hw instead of clk
Suggested-by: Stephen Boyd
v4: use const for capture snd_soc_pcm_stream instance
Suggested-by: Stephen Boyd
---
drivers/media/i2c/adv748x/Makefile | 3 +-
drivers/media/i2c/adv748x/adv748x-core.c | 9 +-
drivers/media/i2c/adv748x/adv748x-dai.c | 261 +++
drivers/media/i2c/adv748x/adv748x.h | 17 +-
4 files changed, 287 insertions(+), 3 deletions(-)
create mode 100644 drivers/media/i2c/adv748x/adv748x-dai.c
diff --git a/drivers/media/i2c/adv748x/Makefile
b/drivers/media/i2c/adv748x/Makefile
index 93844f14cb10..6e7a302ef199 100644
--- a/drivers/media/i2c/adv748x/Makefile
+++ b/drivers/media/i2c/adv748x/Makefile
@@ -3,6 +3,7 @@ adv748x-objs:= \
adv748x-afe.o \
adv748x-core.o \
adv748x-csi2.o \
- adv748x-hdmi.o
+ adv748x-hdmi.o \
+ adv748x-dai.o
obj-$(CONFIG_VIDEO_ADV748X)+= adv748x.o
diff --git a/drivers/media/i2c/adv748x/adv748x-core.c
b/drivers/media/i2c/adv748x/adv748x-core.c
index 8580e6624276..3513ca138e53 100644
--- a/drivers/media/i2c/adv748x/adv748x-core.c
+++ b/drivers/media/i2c/adv748x/adv748x-core.c
@@ -765,8 +765,14 @@ static int adv748x_probe(struct i2c_client *client)
goto err_cleanup_txa;
}
+ ret = adv748x_dai_init(&state->dai);
+ if (ret) {
+ adv_err(state, "Failed to probe DAI\n");
+ goto err_cleanup_txb;
+ }
return 0;
-
+err_cleanup_txb:
+ adv748x_csi2_cleanup(&state->txb);
err_cleanup_txa:
adv748x_csi2_cleanup(&state->txa);
err_cleanup_afe:
@@ -787,6 +793,7 @@ static int adv748x_remove(struct i2c_client *client)
{
struct adv748x_state *state = i2c_get_clientdata(client);
+ adv748x_dai_cleanup(&state->dai);
adv748x_afe_cleanup(&state->afe);
adv748x_hdmi_cleanup(&state->hdmi);
diff --git a/drivers/media/i2c/adv748x/adv748x-dai.c
b/drivers/media/i2c/adv748x/adv748x-dai.c
new file mode 100644
index ..c9191f8f1ca8
--- /dev/null
+++ b/drivers/media/i2c/adv748x/adv748x-dai.c
@@ -0,0 +1,261 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Driver for Analog Devices ADV748X HDMI receiver with AFE
+ * The implementation of DAI.
+ */
+
+#include "adv748x.h"
+
+#include
+#include
+#include
+
+#define state_of(soc_dai) \
+ adv748x_dai_to_state(container_of((soc_dai)->driver, \
+ struct adv748x_dai, drv))
+#define mclk_of(state) ((state)->dai.mclk_hw->clk)
+
+static const char ADV748X_DAI_NAME[] = "adv748x-i2s";
+
+static int set_audio_pads_state(struct adv748x_state *state, int on)
+{
+ return io_clrset(state, ADV748X_IO_PAD_CONTROLS,
+ADV748X_IO_PAD_CONTROLS_TRI_AUD |
+ADV748X_IO_PAD_CONTROLS_PDN_AUD,
+on ? 0 : 0xff);
+}
+
+static int set_dpll_mclk_fs(struct adv748x_state *state, int fs)
+{
+ return dpll_clrset(state, ADV748X_DPLL_MCLK_FS,
+ ADV748X_DPLL_MCLK_FS_N_MASK, (fs / 128) - 1);
+}
+
+static int set_i2s_format(struct adv748x_state *state, uint outmode,
+ uint bitwidth)
+{
+ return hdmi_clrset(state, ADV748X_HDMI_I2S,
+ ADV748X_HDMI_I2SBITWIDTH_MASK |
+ ADV748X_HDMI_I2SOUTMODE_MASK,
+ (outmode << ADV748X_HDMI_I2SOUTMODE_SHIFT) |
+ bitwidth);
+}
+
+static int set_i2s_tdm_mode(struct adv748x_state *state, int is_tdm)
+{
+ int ret;
+
+ ret = hdmi_clrset(state, ADV748X_HDMI_AUDIO_MUTE_SPEED,
+ ADV748X_MAN_AUDIO_DL_BYPASS |
+ ADV748X_AUDIO_DELAY_LINE_BYPASS,
+ is_tdm ? 0xff : 0);
+ if (ret < 0)
+ return ret;
+ ret = hdmi_clrset(state, ADV748X_HDMI_REG_6D,
+ ADV748X_I2S_TDM_MODE_ENABLE,
+ is_tdm ? 0xff : 0);
+ return ret;
+}
+
+static int set_audio_mute(struct adv748x_state *state, int enable)
+{
+ return hdmi_clrset(state, ADV748X_HDMI_MUTE_CTRL,
+ ADV748X_HDMI_MUTE_CTRL_MUTE_AUDIO,
+ enable ? 0xff : 0);
+}
+
+static int adv748x_dai_set_sysclk(struct snd_soc_dai *dai,
+ int clk_id, unsigned int freq, int dir)
+{
+ struct adv748x_state *state = state_of(dai);
+
+ /* currently supporting only one fixed rate clock */
+ if (clk_id != 0 || freq != clk_get_rate(mclk_of(state))) {
+ dev_err(dai->dev, "invalid clock (%d) or frequency (%u, dir
%d)\n",
+ clk_id, freq, dir);
[PATCH v5 6/9] media: adv748x: prepare/enable mclk when the audio is used
As there is nothing else (the consumers are supposed to do that) which
enables the clock, do it in the driver.
Signed-off-by: Alexander Riesen
--
v3: added
---
drivers/media/i2c/adv748x/adv748x-dai.c | 14 +-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/drivers/media/i2c/adv748x/adv748x-dai.c
b/drivers/media/i2c/adv748x/adv748x-dai.c
index c9191f8f1ca8..185f78023e91 100644
--- a/drivers/media/i2c/adv748x/adv748x-dai.c
+++ b/drivers/media/i2c/adv748x/adv748x-dai.c
@@ -117,11 +117,22 @@ static int adv748x_dai_set_fmt(struct snd_soc_dai *dai,
unsigned int fmt)
static int adv748x_dai_startup(struct snd_pcm_substream *sub, struct
snd_soc_dai *dai)
{
+ int ret;
struct adv748x_state *state = state_of(dai);
if (sub->stream != SNDRV_PCM_STREAM_CAPTURE)
return -EINVAL;
- return set_audio_pads_state(state, 1);
+ ret = set_audio_pads_state(state, 1);
+ if (ret)
+ goto fail;
+ ret = clk_prepare_enable(mclk_of(state));
+ if (ret)
+ goto fail_pwdn;
+ return 0;
+fail_pwdn:
+ set_audio_pads_state(state, 0);
+fail:
+ return ret;
}
static int adv748x_dai_hw_params(struct snd_pcm_substream *sub,
@@ -174,6 +185,7 @@ static void adv748x_dai_shutdown(struct snd_pcm_substream
*sub, struct snd_soc_d
{
struct adv748x_state *state = state_of(dai);
+ clk_disable_unprepare(mclk_of(state));
set_audio_pads_state(state, 0);
}
--
2.25.1.25.g9ecbe7eb18
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v5 7/9] media: adv748x: only activate DAI if it is described in device tree
To avoid setting it up even if the hardware is not actually connected
to anything physically.
Besides, the bindings explicitly notes that port definitions are
"optional if they are not connected to anything at the hardware level".
Signed-off-by: Alexander Riesen
---
drivers/media/i2c/adv748x/adv748x-dai.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/drivers/media/i2c/adv748x/adv748x-dai.c
b/drivers/media/i2c/adv748x/adv748x-dai.c
index 185f78023e91..f9cc47fa9ad1 100644
--- a/drivers/media/i2c/adv748x/adv748x-dai.c
+++ b/drivers/media/i2c/adv748x/adv748x-dai.c
@@ -216,6 +216,11 @@ int adv748x_dai_init(struct adv748x_dai *dai)
int ret;
struct adv748x_state *state = adv748x_dai_to_state(dai);
+ if (!state->endpoints[ADV748X_PORT_I2S]) {
+ adv_info(state, "no I2S port, DAI disabled\n");
+ ret = 0;
+ goto fail;
+ }
dai->mclk_name = kasprintf(GFP_KERNEL, "%s.%s-i2s-mclk",
state->dev->driver->name,
dev_name(state->dev));
--
2.25.1.25.g9ecbe7eb18
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v5 8/9] dt-bindings: adv748x: add information about serial audio interface (I2S/TDM)
As the driver has some support for the audio interface of the device,
the bindings file should mention it.
Signed-off-by: Alexander Riesen
Reviewed-by: Rob Herring
Reviewed-by: Laurent Pinchart
--
v3: remove optionality off MCLK clock cell to ensure the description
matches the hardware no matter if the line is connected.
Suggested-by: Geert Uytterhoeven
---
.../devicetree/bindings/media/i2c/adv748x.txt| 16 +++-
1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/media/i2c/adv748x.txt
b/Documentation/devicetree/bindings/media/i2c/adv748x.txt
index 4f91686e54a6..50a753189b81 100644
--- a/Documentation/devicetree/bindings/media/i2c/adv748x.txt
+++ b/Documentation/devicetree/bindings/media/i2c/adv748x.txt
@@ -2,7 +2,9 @@
The ADV7481 and ADV7482 are multi format video decoders with an integrated
HDMI receiver. They can output CSI-2 on two independent outputs TXA and TXB
-from three input sources HDMI, analog and TTL.
+from three input sources HDMI, analog and TTL. There is also support for an
+I2S-compatible interface connected to the audio processor of the HDMI decoder.
+The interface has TDM capability (8 slots, 32 bits, left or right justified).
Required Properties:
@@ -16,6 +18,8 @@ Required Properties:
slave device on the I2C bus. The main address is mandatory, others are
optional and remain at default values if not specified.
+ - #clock-cells: must be <0>
+
Optional Properties:
- interrupt-names: Should specify the interrupts as "intrq1", "intrq2" and/or
@@ -47,6 +51,7 @@ are numbered as follows.
TTL sink9
TXA source 10
TXB source 11
+ I2S source 12
The digital output port nodes, when present, shall contain at least one
endpoint. Each of those endpoints shall contain the data-lanes property as
@@ -72,6 +77,7 @@ Example:
#address-cells = <1>;
#size-cells = <0>;
+ #clock-cells = <0>;
interrupt-parent = <&gpio6>;
interrupt-names = "intrq1", "intrq2";
@@ -113,4 +119,12 @@ Example:
remote-endpoint = <&csi20_in>;
};
};
+
+ port@c {
+ reg = <12>;
+
+ adv7482_i2s: endpoint {
+ remote-endpoint = <&i2s_in>;
+ };
+ };
};
--
2.25.1.25.g9ecbe7eb18
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v5 9/9] arm64: dts: renesas: salvator: add a connection from adv748x codec (HDMI input) to the R-Car SoC
As all known variants of the Salvator board have the HDMI decoder
chip (the ADV7482) connected to the SSI4 on R-Car SoC, the ADV7482
endpoint and the connection definitions are placed in the common board
file.
For the same reason, the CLK_C clock line and I2C configuration (similar
to the ak4613, on the same interface) are added into the common file.
Signed-off-by: Alexander Riesen
--
v5: Add dummy ssi4 node to the rcar sound card in r8a77961, as the
devices (Salvator-X 2nd version with R-Car M3 W+) also reference
salvator-common.dtsi.
Suggested-by: Geert Uytterhoeven
v2: Also add ssi4_ctrl pin group in the sound pins. The pins are
responsible for SCK4 (sample clock) WS4 and (word boundary input),
and are required for SSI audio input over I2S.
The adv748x shall provide its own implementation of the output clock
(MCLK), connected to the audio_clk_c line of the R-Car SoC.
If the frequency of the ADV748x MCLK were fixed, the clock
implementation were not necessary, but it does not seem so: the MCLK
depends on the value in a speed multiplier register and the input sample
rate (48kHz).
Remove audio clock C from the clocks of adv7482.
The clocks property of the video-receiver node lists the input
clocks of the device, which is quite the opposite from the
original intention: the adv7482 on Salvator X boards is a
provide of the MCLK clock for I2S audio output.
Remove old definition of &sound_card.dais and reduce size of changes
in the Salvator-X specific device tree source.
Declare video-receiver a clock producer, as the adv748x driver
implements the master clock used I2S audio output.
Suggested-by: Geert Uytterhoeven
v2: The driver provides only MCLK clock, not the SCLK and LRCLK,
which are part of the I2S protocol.
Suggested-by: Laurent Pinchart
---
.../boot/dts/renesas/r8a77950-salvator-x.dts | 3 +-
arch/arm64/boot/dts/renesas/r8a77961.dtsi | 1 +
.../boot/dts/renesas/salvator-common.dtsi | 47 +--
3 files changed, 45 insertions(+), 6 deletions(-)
diff --git a/arch/arm64/boot/dts/renesas/r8a77950-salvator-x.dts
b/arch/arm64/boot/dts/renesas/r8a77950-salvator-x.dts
index 2438825c9b22..e16c146808b6 100644
--- a/arch/arm64/boot/dts/renesas/r8a77950-salvator-x.dts
+++ b/arch/arm64/boot/dts/renesas/r8a77950-salvator-x.dts
@@ -146,7 +146,8 @@ &sata {
&sound_card {
dais = <&rsnd_port0 /* ak4613 */
&rsnd_port1 /* HDMI0 */
- &rsnd_port2>; /* HDMI1 */
+ &rsnd_port2 /* HDMI1 */
+ &rsnd_port3>; /* adv7482 hdmi-in */
};
&usb2_phy2 {
diff --git a/arch/arm64/boot/dts/renesas/r8a77961.dtsi
b/arch/arm64/boot/dts/renesas/r8a77961.dtsi
index be3824bda632..b79907beaf31 100644
--- a/arch/arm64/boot/dts/renesas/r8a77961.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a77961.dtsi
@@ -861,6 +861,7 @@ rcar_sound,src {
rcar_sound,ssi {
ssi0: ssi-0 { };
ssi1: ssi-1 { };
+ ssi4: ssi-4 { };
};
};
diff --git a/arch/arm64/boot/dts/renesas/salvator-common.dtsi
b/arch/arm64/boot/dts/renesas/salvator-common.dtsi
index 98bbcafc8c0d..ead7f8d7a929 100644
--- a/arch/arm64/boot/dts/renesas/salvator-common.dtsi
+++ b/arch/arm64/boot/dts/renesas/salvator-common.dtsi
@@ -460,7 +460,7 @@ pca9654: gpio@20 {
#gpio-cells = <2>;
};
- video-receiver@70 {
+ adv7482_hdmi_in: video-receiver@70 {
compatible = "adi,adv7482";
reg = <0x70 0x71 0x72 0x73 0x74 0x75
0x60 0x61 0x62 0x63 0x64 0x65>;
@@ -469,6 +469,7 @@ video-receiver@70 {
#address-cells = <1>;
#size-cells = <0>;
+ #clock-cells = <0>; /* the MCLK for I2S output */
interrupt-parent = <&gpio6>;
interrupt-names = "intrq1", "intrq2";
@@ -510,6 +511,15 @@ adv7482_txb: endpoint {
remote-endpoint = <&csi20_in>;
};
};
+
+ port@c {
+ reg = <12>;
+
+ adv7482_i2s: endpoint {
+ remote-endpoint = <&rsnd_endpoint3>;
+ system-clock-direction-out;
+ };
+ };
};
csa_vdd: adc@7c {
@@ -684,7 +694,8 @@ sdhi3_pins_uhs: sd3_uhs {
};
sound_pins: sound {
- groups = "ssi01239_ctrl", "ssi0_data", "ssi1_data_a";
+ groups = "ssi01239_ctrl", "ssi0_data", "ssi1_data_a",
+"ssi4_data", "ssi4_ctrl";
function = "ssi";
};
@@ -733,8 +744,8 @@ &rcar_sound {
pinctrl-0 = <&sound_pins &sound_clk_pins>;
pinctrl-names = "default";
- /* Si
Re: [PATCH 4/4] arm64: dts: rockchip: add isp0 node for rk3399
On 4/2/20 2:20 PM, Johan Jonker wrote:
> Hi Helen,
>
>> diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
>> b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
>> index fc0295d2a65a1..815099a0cd0dd 100644
>> --- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
>> +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
>> @@ -1718,6 +1718,33 @@ vopb_mmu: iommu@ff903f00 {
>> status = "disabled";
>> };
>>
>> +isp0: isp0@ff91 {
>> +compatible = "rockchip,rk3399-cif-isp";
>> +reg = <0x0 0xff91 0x0 0x4000>;
>> +interrupts = ;
>> +clocks = <&cru SCLK_ISP0>,
>> + <&cru ACLK_ISP0>, <&cru ACLK_ISP0_WRAPPER>,
>> + <&cru HCLK_ISP0>, <&cru HCLK_ISP0_WRAPPER>;
>> +clock-names = "clk_isp",
>> + "aclk_isp", "aclk_isp_wrap",
>> + "hclk_isp", "hclk_isp_wrap";
>
>> +power-domains = <&power RK3399_PD_ISP0>;
>> +iommus = <&isp0_mmu>;
>> +phys = <&mipi_dphy_rx0>;
>> +phy-names = "dphy";
>
> Maybe a little sort? But keep rest as it is. Also in example.
>
> iommus = <&isp0_mmu>;
> phys = <&mipi_dphy_rx0>;
> phy-names = "dphy";
> power-domains = <&power RK3399_PD_ISP0>;
Are you proposing only to move power-domains after phy? And keep the rest?
What is the main logic?
Thanks
Helen
>
>> +
>> +ports {
>> +#address-cells = <1>;
>> +#size-cells = <0>;
>> +
>> +port@0 {
>
>> +#address-cells = <1>;
>> +#size-cells = <0>;
>> +reg = <0>;
>
> Move reg above #address-cells. Change that in example as well.
>
> reg = <0>;
> #address-cells = <1>;
> #size-cells = <0>;
>
>> +};
>> +};
>> +};
>> +
>> isp0_mmu: iommu@ff914000 {
>> compatible = "rockchip,iommu";
>> reg = <0x0 0xff914000 0x0 0x100>, <0x0 0xff915000 0x0 0x100>;
>> --
>> 2.26.0
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 4/4] arm64: dts: rockchip: add isp0 node for rk3399
On 4/2/20 9:46 PM, Helen Koike wrote:
>
>
> On 4/2/20 2:20 PM, Johan Jonker wrote:
>> Hi Helen,
>>
>>> diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
>>> b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
>>> index fc0295d2a65a1..815099a0cd0dd 100644
>>> --- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
>>> +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
>>> @@ -1718,6 +1718,33 @@ vopb_mmu: iommu@ff903f00 {
>>> status = "disabled";
>>> };
>>>
>>> + isp0: isp0@ff91 {
>>> + compatible = "rockchip,rk3399-cif-isp";
>>> + reg = <0x0 0xff91 0x0 0x4000>;
>>> + interrupts = ;
>>> + clocks = <&cru SCLK_ISP0>,
>>> +<&cru ACLK_ISP0>, <&cru ACLK_ISP0_WRAPPER>,
>>> +<&cru HCLK_ISP0>, <&cru HCLK_ISP0_WRAPPER>;
>>> + clock-names = "clk_isp",
>>> + "aclk_isp", "aclk_isp_wrap",
>>> + "hclk_isp", "hclk_isp_wrap";
>>
>>> + power-domains = <&power RK3399_PD_ISP0>;
>>> + iommus = <&isp0_mmu>;
>>> + phys = <&mipi_dphy_rx0>;
>>> + phy-names = "dphy";
>>
>> Maybe a little sort? But keep rest as it is. Also in example.
>>
>> iommus = <&isp0_mmu>;
>> phys = <&mipi_dphy_rx0>;
>> phy-names = "dphy";
>> power-domains = <&power RK3399_PD_ISP0>;
>
> Are you proposing only to move power-domains after phy? And keep the rest?
> What is the main logic?
There is no hard rule... It mostly depend on Heiko...
For nodes:
Sort things without reg alphabetical first,
then sort the rest by reg address.
Inside nodes:
If exists on top: compatible, reg and interrupts.
In alphabetical order the required properties.
Then in alphabetical order the other properties.
And as last things that start with '#' in alphabetical order.
>
> Thanks
> Helen
>
>>
>>> +
>>> + ports {
>>> + #address-cells = <1>;
>>> + #size-cells = <0>;
>>> +
>>> + port@0 {
>>
>>> + #address-cells = <1>;
>>> + #size-cells = <0>;
>>> + reg = <0>;
>>
>> Move reg above #address-cells. Change that in example as well.
>>
>> reg = <0>;
>> #address-cells = <1>;
>> #size-cells = <0>;
>>
>>> + };
>>> + };
>>> + };
>>> +
>>> isp0_mmu: iommu@ff914000 {
>>> compatible = "rockchip,iommu";
>>> reg = <0x0 0xff914000 0x0 0x100>, <0x0 0xff915000 0x0 0x100>;
>>> --
>>> 2.26.0
>>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: vt6656: replace al2230_power_table array with formula.
The power table can replaced with calculation 0x0404090 | (power << 12)
removing array and length macro.
variable power never goes beyond the maximum setting.
Signed-off-by: Malcolm Priestley
---
drivers/staging/vt6656/rf.c | 79 ++---
1 file changed, 4 insertions(+), 75 deletions(-)
diff --git a/drivers/staging/vt6656/rf.c b/drivers/staging/vt6656/rf.c
index 43237b7e1dbe..4f9aba0f21b0 100644
--- a/drivers/staging/vt6656/rf.c
+++ b/drivers/staging/vt6656/rf.c
@@ -27,7 +27,6 @@
#include "usbpipe.h"
#define CB_AL2230_INIT_SEQ15
-#define AL2230_PWR_IDX_LEN64
#define CB_AL7230_INIT_SEQ16
#define AL7230_PWR_IDX_LEN64
@@ -518,74 +517,6 @@ static u8 vt3342_channel_table1[CB_MAX_CHANNEL][3] = {
{0x03, 0x00, 0x04}
};
-/* Power Table */
-static const u32 al2230_power_table[AL2230_PWR_IDX_LEN] = {
- 0x04040900,
- 0x04041900,
- 0x04042900,
- 0x04043900,
- 0x04044900,
- 0x04045900,
- 0x04046900,
- 0x04047900,
- 0x04048900,
- 0x04049900,
- 0x0404a900,
- 0x0404b900,
- 0x0404c900,
- 0x0404d900,
- 0x0404e900,
- 0x0404f900,
- 0x04050900,
- 0x04051900,
- 0x04052900,
- 0x04053900,
- 0x04054900,
- 0x04055900,
- 0x04056900,
- 0x04057900,
- 0x04058900,
- 0x04059900,
- 0x0405a900,
- 0x0405b900,
- 0x0405c900,
- 0x0405d900,
- 0x0405e900,
- 0x0405f900,
- 0x04060900,
- 0x04061900,
- 0x04062900,
- 0x04063900,
- 0x04064900,
- 0x04065900,
- 0x04066900,
- 0x04067900,
- 0x04068900,
- 0x04069900,
- 0x0406a900,
- 0x0406b900,
- 0x0406c900,
- 0x0406d900,
- 0x0406e900,
- 0x0406f900,
- 0x04070900,
- 0x04071900,
- 0x04072900,
- 0x04073900,
- 0x04074900,
- 0x04075900,
- 0x04076900,
- 0x04077900,
- 0x04078900,
- 0x04079900,
- 0x0407a900,
- 0x0407b900,
- 0x0407c900,
- 0x0407d900,
- 0x0407e900,
- 0x0407f900
-};
-
/*
* Description: Write to IF/RF, by embedded programming
*/
@@ -685,10 +616,9 @@ int vnt_rf_set_txpower(struct vnt_private *priv, u8 power,
u32 rate)
switch (priv->rf_type) {
case RF_AL2230:
- if (power >= AL2230_PWR_IDX_LEN)
- return false;
+ power_setting = 0x0404090 | (power << 12);
- ret &= vnt_rf_write_embedded(priv, al2230_power_table[power]);
+ ret &= vnt_rf_write_embedded(priv, power_setting);
if (rate <= RATE_11M)
ret &= vnt_rf_write_embedded(priv, 0x0001b400);
@@ -696,10 +626,9 @@ int vnt_rf_set_txpower(struct vnt_private *priv, u8 power,
u32 rate)
ret &= vnt_rf_write_embedded(priv, 0x0005a400);
break;
case RF_AL2230S:
- if (power >= AL2230_PWR_IDX_LEN)
- return false;
+ power_setting = 0x0404090 | (power << 12);
- ret &= vnt_rf_write_embedded(priv, al2230_power_table[power]);
+ ret &= vnt_rf_write_embedded(priv, power_setting);
if (rate <= RATE_11M) {
ret &= vnt_rf_write_embedded(priv, 0x040c1400);
--
2.25.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/2] staging: vt6656: set all ofdm rates to default
mac80211 rate control decides which odfm rates to use so all of
them should be set enabled at the appropriate bit rate.
This means vnt_get_ofdm_rate is no longer required.
Signed-off-by: Malcolm Priestley
---
drivers/staging/vt6656/card.c | 54 +++
1 file changed, 4 insertions(+), 50 deletions(-)
diff --git a/drivers/staging/vt6656/card.c b/drivers/staging/vt6656/card.c
index dc3ab10eb630..4abbe9b30b65 100644
--- a/drivers/staging/vt6656/card.c
+++ b/drivers/staging/vt6656/card.c
@@ -100,48 +100,6 @@ static u16 vnt_get_cck_rate(struct vnt_private *priv, u16
rate_idx)
return RATE_1M;
}
-/*
- * Description: Get OFDM mode basic rate
- *
- * Parameters:
- * In:
- * priv - The adapter to be set
- * rate_idx - Receiving data rate
- * Out:
- * none
- *
- * Return Value: response Control frame rate
- *
- */
-static u16 vnt_get_ofdm_rate(struct vnt_private *priv, u16 rate_idx)
-{
- u16 ui = rate_idx;
-
- dev_dbg(&priv->usb->dev, "%s basic rate: %d\n",
- __func__, priv->basic_rates);
-
- if (!vnt_ofdm_min_rate(priv)) {
- dev_dbg(&priv->usb->dev, "%s (NO OFDM) %d\n",
- __func__, rate_idx);
- if (rate_idx > RATE_24M)
- rate_idx = RATE_24M;
- return rate_idx;
- }
-
- while (ui > RATE_11M) {
- if (priv->basic_rates & (1 << ui)) {
- dev_dbg(&priv->usb->dev, "%s rate: %d\n",
- __func__, ui);
- return ui;
- }
- ui--;
- }
-
- dev_dbg(&priv->usb->dev, "%s basic rate: 24M\n", __func__);
-
- return RATE_24M;
-}
-
/*
* Description: Calculate TxRate and RsvTime fields for RSPINF in OFDM mode.
*
@@ -289,20 +247,16 @@ void vnt_set_rspinf(struct vnt_private *priv, u8 bb_type)
vnt_calculate_ofdm_rate(RATE_24M, bb_type, &tx_rate[4], &rsv_time[4]);
/*RSPINF_a_36*/
- vnt_calculate_ofdm_rate(vnt_get_ofdm_rate(priv, RATE_36M),
- bb_type, &tx_rate[5], &rsv_time[5]);
+ vnt_calculate_ofdm_rate(RATE_36M, bb_type, &tx_rate[5], &rsv_time[5]);
/*RSPINF_a_48*/
- vnt_calculate_ofdm_rate(vnt_get_ofdm_rate(priv, RATE_48M),
- bb_type, &tx_rate[6], &rsv_time[6]);
+ vnt_calculate_ofdm_rate(RATE_48M, bb_type, &tx_rate[6], &rsv_time[6]);
/*RSPINF_a_54*/
- vnt_calculate_ofdm_rate(vnt_get_ofdm_rate(priv, RATE_54M),
- bb_type, &tx_rate[7], &rsv_time[7]);
+ vnt_calculate_ofdm_rate(RATE_54M, bb_type, &tx_rate[7], &rsv_time[7]);
/*RSPINF_a_72*/
- vnt_calculate_ofdm_rate(vnt_get_ofdm_rate(priv, RATE_54M),
- bb_type, &tx_rate[8], &rsv_time[8]);
+ vnt_calculate_ofdm_rate(RATE_54M, bb_type, &tx_rate[8], &rsv_time[8]);
put_unaligned(phy[0].len, (u16 *)&data[0]);
data[2] = phy[0].signal;
--
2.25.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/2] staging: vt6656: set all cck rates to default.
mac80211 rate control decides which cck rates to use so all of
them should be set enabled at the appropriate bit rate.
This means vnt_get_cck_rate is no longer required.
Signed-off-by: Malcolm Priestley
---
drivers/staging/vt6656/card.c | 38 ---
1 file changed, 4 insertions(+), 34 deletions(-)
diff --git a/drivers/staging/vt6656/card.c b/drivers/staging/vt6656/card.c
index 4abbe9b30b65..97e1538a528e 100644
--- a/drivers/staging/vt6656/card.c
+++ b/drivers/staging/vt6656/card.c
@@ -74,32 +74,6 @@ void vnt_set_channel(struct vnt_private *priv, u32
connection_channel)
(u8)(connection_channel | 0x80));
}
-/*
- * Description: Get CCK mode basic rate
- *
- * Parameters:
- * In:
- * priv - The adapter to be set
- * rate_idx - Receiving data rate
- * Out:
- * none
- *
- * Return Value: response Control frame rate
- *
- */
-static u16 vnt_get_cck_rate(struct vnt_private *priv, u16 rate_idx)
-{
- u16 ui = rate_idx;
-
- while (ui > RATE_1M) {
- if (priv->basic_rates & (1 << ui))
- return ui;
- ui--;
- }
-
- return RATE_1M;
-}
-
/*
* Description: Calculate TxRate and RsvTime fields for RSPINF in OFDM mode.
*
@@ -216,20 +190,16 @@ void vnt_set_rspinf(struct vnt_private *priv, u8 bb_type)
int i;
/*RSPINF_b_1*/
- vnt_get_phy_field(priv, 14, vnt_get_cck_rate(priv, RATE_1M),
- PK_TYPE_11B, &phy[0]);
+ vnt_get_phy_field(priv, 14, RATE_1M, PK_TYPE_11B, &phy[0]);
/*RSPINF_b_2*/
- vnt_get_phy_field(priv, 14, vnt_get_cck_rate(priv, RATE_2M),
- PK_TYPE_11B, &phy[1]);
+ vnt_get_phy_field(priv, 14, RATE_2M, PK_TYPE_11B, &phy[1]);
/*RSPINF_b_5*/
- vnt_get_phy_field(priv, 14, vnt_get_cck_rate(priv, RATE_5M),
- PK_TYPE_11B, &phy[2]);
+ vnt_get_phy_field(priv, 14, RATE_5M, PK_TYPE_11B, &phy[2]);
/*RSPINF_b_11*/
- vnt_get_phy_field(priv, 14, vnt_get_cck_rate(priv, RATE_11M),
- PK_TYPE_11B, &phy[3]);
+ vnt_get_phy_field(priv, 14, RATE_11M, PK_TYPE_11B, &phy[3]);
/*RSPINF_a_6*/
vnt_calculate_ofdm_rate(RATE_6M, bb_type, &tx_rate[0], &rsv_time[0]);
--
2.25.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
FROM ABDUL KAREEM
I am Abdul Kareem working with Emirate NBD Bank Dubai,United Arab Emirate as Sales and Marketing Officer,I have a business proposal concerning you and will benefit we both financially,Kindly email me now for more details. Awaiting your response. Thanks Abdul Kareem ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [Outreachy kernel] [PATCH] staging: emxx_udc: Remove unused code
On Wed, 1 Apr 2020 19:17:06 -0700 "John B. Wyatt IV" wrote: > Remove unused code surrounded by an #if 0 block. > > Code has not been altered since 2014 as reported by git blame. > > Reported by checkpatch. > > Signed-off-by: John B. Wyatt IV > --- > drivers/staging/emxx_udc/emxx_udc.h | 6 -- > 1 file changed, 6 deletions(-) > > diff --git a/drivers/staging/emxx_udc/emxx_udc.h > b/drivers/staging/emxx_udc/emxx_udc.h > index 9c2671cb32f7..bbfebe331033 100644 > --- a/drivers/staging/emxx_udc/emxx_udc.h > +++ b/drivers/staging/emxx_udc/emxx_udc.h > @@ -9,12 +9,6 @@ > #define _LINUX_EMXX_H > > > /*---*/ > -/*- Default undef */ > -#if 0 > -#define DEBUG > -#define UDC_DEBUG_DUMP > -#endif > - > /*- Default define */ > #define USE_DMA 1 > #define USE_SUSPEND_WAIT 1 Formally, this is fine. But... think about it: this driver might be rather buggy, so the first thing one might want to do with it is to "enable" those two defines. In general, that stuff has to disappear, and proper debugging facilities have to be used, but with a driver in this state, as long as proper debugging facilities aren't there, you might be doing more harm than good. -- Stefano ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v3 0/5] staging: rtl8712: rtl871x_xmit.{c, h} code style improvements
Thanks Dan for your review!! On Thu, Apr 02, 2020 at 01:29:06PM +0300, Dan Carpenter wrote: > Looks good. Thanks! > > Reviewed-by: Dan Carpenter > > regards, > dan carpenter > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [Outreachy kernel] [PATCH] staging: emxx_udc: Remove unused code
On Fri, 2020-04-03 at 01:50 +0200, Stefano Brivio wrote: > On Wed, 1 Apr 2020 19:17:06 -0700 > "John B. Wyatt IV" wrote: > > > Remove unused code surrounded by an #if 0 block. > > > > Code has not been altered since 2014 as reported by git blame. > > > > Reported by checkpatch. > > > > Signed-off-by: John B. Wyatt IV > > --- > > drivers/staging/emxx_udc/emxx_udc.h | 6 -- > > 1 file changed, 6 deletions(-) > > > > diff --git a/drivers/staging/emxx_udc/emxx_udc.h > > b/drivers/staging/emxx_udc/emxx_udc.h > > index 9c2671cb32f7..bbfebe331033 100644 > > --- a/drivers/staging/emxx_udc/emxx_udc.h > > +++ b/drivers/staging/emxx_udc/emxx_udc.h > > @@ -9,12 +9,6 @@ > > #define _LINUX_EMXX_H > > > > /*-- > > -*/ > > -/*- Default undef */ > > -#if 0 > > -#define DEBUG > > -#define UDC_DEBUG_DUMP > > -#endif > > - > > /*- Default define */ > > #defineUSE_DMA 1 > > #define USE_SUSPEND_WAIT 1 > > Formally, this is fine. But... think about it: this driver might be > rather buggy, so the first thing one might want to do with it is to > "enable" those two defines. > > In general, that stuff has to disappear, and proper debugging > facilities have to be used, but with a driver in this state, as long > as > proper debugging facilities aren't there, you might be doing more > harm > than good. DEBUG is not actually used as far as I can tell (I am still new to kernel debugging systems to please correct me). There is only a pair of .c and .h files for this small driver. UDC_DEBUG_DUMP is only used twice in the entire kernel-both for if statements. Should we just set it to: #define UDC_DEBUG_DUMP 0 And leave the other 3 lines out? Please let me know for a v2. > > -- > Stefano > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2] staging: android: ion: Align with parenthesis
Align two different lines of arguments with the parenthesis of their respected function definitions. Fix style warnings of matching alignment. Reported by checkpatch. Signed-off-by: John B. Wyatt IV --- v2: Change comment title and summary Suggested-by: Julia Lawall drivers/staging/android/ion/ion_page_pool.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/staging/android/ion/ion_page_pool.c b/drivers/staging/android/ion/ion_page_pool.c index f85ec5b16b65..0198b886d906 100644 --- a/drivers/staging/android/ion/ion_page_pool.c +++ b/drivers/staging/android/ion/ion_page_pool.c @@ -37,7 +37,7 @@ static void ion_page_pool_add(struct ion_page_pool *pool, struct page *page) } mod_node_page_state(page_pgdat(page), NR_KERNEL_MISC_RECLAIMABLE, - 1 << pool->order); + 1 << pool->order); mutex_unlock(&pool->mutex); } @@ -57,7 +57,7 @@ static struct page *ion_page_pool_remove(struct ion_page_pool *pool, bool high) list_del(&page->lru); mod_node_page_state(page_pgdat(page), NR_KERNEL_MISC_RECLAIMABLE, - -(1 << pool->order)); + -(1 << pool->order)); return page; } -- 2.25.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [Outreachy kernel] [PATCH] staging: emxx_udc: Remove unused code
On Thu, 2020-04-02 at 18:42 -0700, John B. Wyatt IV wrote: > DEBUG is not actually used as far as I can tell (I am still new to > kernel debugging systems to please correct me). DEBUG is used by the _debug and _dbg macros (pr_debug, dev_dbg) ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Please Help People Around You?
My name is Reem E. Al-Hashimi, the Emirates Minister of State and Managing Director of United Arab Emirates (Dubai) World Expo 2020 Committee. I am writing you to stand as my partner to receive my share of gratification from foreign companies whom I helped during the bidding exercise towards the Dubai World Expo 2020 Committee and also i want to use this funds assist Corona virus Symptoms and Causes. Am a single Arab women and serving as a minister, there is a limit to my personal income and investment level and For this reason, I cannot receive such a huge sum back to my country or my personal account, so an agreement was reached with the foreign companies to direct the gratifications to an open beneficiary account with a financial institution where it will be possible for me to instruct further transfer of the fund to a third party account for investment purpose which is the reason i contacted you to receive the fund as my partner for investment in your country. The amount is valued at Euro 47,745,533.00 with a financial institution waiting my instruction for further transfer to a destination account as soon as I have your information indicating interest to receive and invest the fund, I will compensate you with 30% of the total amount and you will also get benefit from the investment. If you can handle the fund in a good investment. reply on this email only: reemalhash...@daum.net Regards, Ms. Reem ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
