[staging:staging-testing] BUILD SUCCESS 8436f932d84b1d53d2f4a2fa88c7aacdb0313265

2020-09-17 Thread kernel test robot 
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git 
 staging-testing
branch HEAD: 8436f932d84b1d53d2f4a2fa88c7aacdb0313265  staging: hikey9xx: 
convert phy-kirin970-usb3.txt to yaml

elapsed time: 722m

configs tested: 105
configs skipped: 2

The following configs have been built successfully.
More configs may be tested in the coming days.

gcc tested configs:
arm defconfig
arm64allyesconfig
arm64   defconfig
arm  allyesconfig
arm  allmodconfig
xtensa   allyesconfig
powerpc  mpc866_ads_defconfig
arcvdk_hs38_smp_defconfig
powerpccell_defconfig
mipsnlm_xlp_defconfig
sh  rsk7269_defconfig
powerpc  ppc6xx_defconfig
arc nps_defconfig
arm   h5000_defconfig
mipsbcm47xx_defconfig
armneponset_defconfig
powerpcgamecube_defconfig
xtensa  iss_defconfig
mips  loongson3_defconfig
mips cobalt_defconfig
powerpc   maple_defconfig
sh  sdk7780_defconfig
powerpc sbc8548_defconfig
mips   capcella_defconfig
powerpc mpc837x_rdb_defconfig
sh  rts7751r2d1_defconfig
powerpc pq2fads_defconfig
um   x86_64_defconfig
powerpc rainier_defconfig
sh  ul2_defconfig
ia64  gensparse_defconfig
openrisc alldefconfig
sh   sh7724_generic_defconfig
sh   j2_defconfig
powerpc   ppc64_defconfig
arm  iop32x_defconfig
m68km5272c3_defconfig
ia64 allmodconfig
ia64defconfig
ia64 allyesconfig
m68k allmodconfig
m68kdefconfig
m68k allyesconfig
nios2   defconfig
arc  allyesconfig
nds32 allnoconfig
c6x  allyesconfig
nds32   defconfig
nios2allyesconfig
cskydefconfig
alpha   defconfig
alphaallyesconfig
h8300allyesconfig
arc defconfig
sh   allmodconfig
parisc  defconfig
s390 allyesconfig
parisc   allyesconfig
s390defconfig
i386 allyesconfig
sparcallyesconfig
sparc   defconfig
i386defconfig
mips allyesconfig
mips allmodconfig
powerpc  allyesconfig
powerpc  allmodconfig
powerpc   allnoconfig
i386 randconfig-a004-20200917
i386 randconfig-a006-20200917
i386 randconfig-a003-20200917
i386 randconfig-a001-20200917
i386 randconfig-a002-20200917
i386 randconfig-a005-20200917
x86_64   randconfig-a014-20200917
x86_64   randconfig-a011-20200917
x86_64   randconfig-a016-20200917
x86_64   randconfig-a012-20200917
x86_64   randconfig-a015-20200917
x86_64   randconfig-a013-20200917
i386 randconfig-a015-20200917
i386 randconfig-a014-20200917
i386 randconfig-a011-20200917
i386 randconfig-a013-20200917
i386 randconfig-a016-20200917
i386 randconfig-a012-20200917
riscvnommu_k210_defconfig
riscvallyesconfig
riscvnommu_virt_defconfig
riscv allnoconfig
riscv   defconfig
riscv  rv32_defconfig
riscvallmodconfig
x86_64   rhel
x86_64   allyesconfig
x86_64rhel-7.6-kselftests
x86_64  defconfig
x86_64   rhel-8.3
x86_64  kexec

clang tested configs:
x86_64

[PATCH AUTOSEL 4.14 090/127] staging:r8188eu: avoid skb_clone for amsdu to msdu conversion

2020-09-17 Thread Sasha Levin 
From: Ivan Safonov 

[ Upstream commit 628cbd971a927abe6388d44320e351c337b331e4 ]

skb clones use same data buffer,
so tail of one skb is corrupted by beginning of next skb.

Signed-off-by: Ivan Safonov 
Link: https://lore.kernel.org/r/20200423191404.12028-1-insafo...@gmail.com
Signed-off-by: Greg Kroah-Hartman 
Signed-off-by: Sasha Levin 
---
 drivers/staging/rtl8188eu/core/rtw_recv.c | 19 ++-
 1 file changed, 6 insertions(+), 13 deletions(-)

diff --git a/drivers/staging/rtl8188eu/core/rtw_recv.c 
b/drivers/staging/rtl8188eu/core/rtw_recv.c
index afb9dadc1cfe9..77685bae21eda 100644
--- a/drivers/staging/rtl8188eu/core/rtw_recv.c
+++ b/drivers/staging/rtl8188eu/core/rtw_recv.c
@@ -1541,21 +1541,14 @@ static int amsdu_to_msdu(struct adapter *padapter, 
struct recv_frame *prframe)
 
/* Allocate new skb for releasing to upper layer */
sub_skb = dev_alloc_skb(nSubframe_Length + 12);
-   if (sub_skb) {
-   skb_reserve(sub_skb, 12);
-   skb_put_data(sub_skb, pdata, nSubframe_Length);
-   } else {
-   sub_skb = skb_clone(prframe->pkt, GFP_ATOMIC);
-   if (sub_skb) {
-   sub_skb->data = pdata;
-   sub_skb->len = nSubframe_Length;
-   skb_set_tail_pointer(sub_skb, nSubframe_Length);
-   } else {
-   DBG_88E("skb_clone() Fail!!! , 
nr_subframes=%d\n", nr_subframes);
-   break;
-   }
+   if (!sub_skb) {
+   DBG_88E("dev_alloc_skb() Fail!!! , nr_subframes=%d\n", 
nr_subframes);
+   break;
}
 
+   skb_reserve(sub_skb, 12);
+   skb_put_data(sub_skb, pdata, nSubframe_Length);
+
subframes[nr_subframes++] = sub_skb;
 
if (nr_subframes >= MAX_SUBFRAME_COUNT) {
-- 
2.25.1

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

[PATCH AUTOSEL 4.14 051/127] media: staging/imx: Missing assignment in imx_media_capture_device_register()

2020-09-17 Thread Sasha Levin 
From: Dan Carpenter 

[ Upstream commit ef0ed05dcef8a74178a8b480cce23a377b1de2b8 ]

There was supposed to be a "ret = " assignment here, otherwise the
error handling on the next line won't work.

Fixes: 64b5a49df486 ("[media] media: imx: Add Capture Device Interface")
Signed-off-by: Dan Carpenter 
Reviewed-by: Steve Longerbeam 
Signed-off-by: Hans Verkuil 
Signed-off-by: Mauro Carvalho Chehab 
Signed-off-by: Sasha Levin 
---
 drivers/staging/media/imx/imx-media-capture.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/media/imx/imx-media-capture.c 
b/drivers/staging/media/imx/imx-media-capture.c
index ea145bafb880a..8ff8843df5141 100644
--- a/drivers/staging/media/imx/imx-media-capture.c
+++ b/drivers/staging/media/imx/imx-media-capture.c
@@ -685,7 +685,7 @@ int imx_media_capture_device_register(struct 
imx_media_video_dev *vdev)
/* setup default format */
fmt_src.pad = priv->src_sd_pad;
fmt_src.which = V4L2_SUBDEV_FORMAT_ACTIVE;
-   v4l2_subdev_call(sd, pad, get_fmt, NULL, _src);
+   ret = v4l2_subdev_call(sd, pad, get_fmt, NULL, _src);
if (ret) {
v4l2_err(sd, "failed to get src_sd format\n");
goto unreg;
-- 
2.25.1

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

[PATCH AUTOSEL 4.19 151/206] staging:r8188eu: avoid skb_clone for amsdu to msdu conversion

2020-09-17 Thread Sasha Levin 
From: Ivan Safonov 

[ Upstream commit 628cbd971a927abe6388d44320e351c337b331e4 ]

skb clones use same data buffer,
so tail of one skb is corrupted by beginning of next skb.

Signed-off-by: Ivan Safonov 
Link: https://lore.kernel.org/r/20200423191404.12028-1-insafo...@gmail.com
Signed-off-by: Greg Kroah-Hartman 
Signed-off-by: Sasha Levin 
---
 drivers/staging/rtl8188eu/core/rtw_recv.c | 19 ++-
 1 file changed, 6 insertions(+), 13 deletions(-)

diff --git a/drivers/staging/rtl8188eu/core/rtw_recv.c 
b/drivers/staging/rtl8188eu/core/rtw_recv.c
index 17b4b9257b495..0ddf41b5a734a 100644
--- a/drivers/staging/rtl8188eu/core/rtw_recv.c
+++ b/drivers/staging/rtl8188eu/core/rtw_recv.c
@@ -1535,21 +1535,14 @@ static int amsdu_to_msdu(struct adapter *padapter, 
struct recv_frame *prframe)
 
/* Allocate new skb for releasing to upper layer */
sub_skb = dev_alloc_skb(nSubframe_Length + 12);
-   if (sub_skb) {
-   skb_reserve(sub_skb, 12);
-   skb_put_data(sub_skb, pdata, nSubframe_Length);
-   } else {
-   sub_skb = skb_clone(prframe->pkt, GFP_ATOMIC);
-   if (sub_skb) {
-   sub_skb->data = pdata;
-   sub_skb->len = nSubframe_Length;
-   skb_set_tail_pointer(sub_skb, nSubframe_Length);
-   } else {
-   DBG_88E("skb_clone() Fail!!! , 
nr_subframes=%d\n", nr_subframes);
-   break;
-   }
+   if (!sub_skb) {
+   DBG_88E("dev_alloc_skb() Fail!!! , nr_subframes=%d\n", 
nr_subframes);
+   break;
}
 
+   skb_reserve(sub_skb, 12);
+   skb_put_data(sub_skb, pdata, nSubframe_Length);
+
subframes[nr_subframes++] = sub_skb;
 
if (nr_subframes >= MAX_SUBFRAME_COUNT) {
-- 
2.25.1

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

[PATCH AUTOSEL 4.19 085/206] media: staging/imx: Missing assignment in imx_media_capture_device_register()

2020-09-17 Thread Sasha Levin 
From: Dan Carpenter 

[ Upstream commit ef0ed05dcef8a74178a8b480cce23a377b1de2b8 ]

There was supposed to be a "ret = " assignment here, otherwise the
error handling on the next line won't work.

Fixes: 64b5a49df486 ("[media] media: imx: Add Capture Device Interface")
Signed-off-by: Dan Carpenter 
Reviewed-by: Steve Longerbeam 
Signed-off-by: Hans Verkuil 
Signed-off-by: Mauro Carvalho Chehab 
Signed-off-by: Sasha Levin 
---
 drivers/staging/media/imx/imx-media-capture.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/media/imx/imx-media-capture.c 
b/drivers/staging/media/imx/imx-media-capture.c
index 256039ce561e6..81a3370551dbc 100644
--- a/drivers/staging/media/imx/imx-media-capture.c
+++ b/drivers/staging/media/imx/imx-media-capture.c
@@ -678,7 +678,7 @@ int imx_media_capture_device_register(struct 
imx_media_video_dev *vdev)
/* setup default format */
fmt_src.pad = priv->src_sd_pad;
fmt_src.which = V4L2_SUBDEV_FORMAT_ACTIVE;
-   v4l2_subdev_call(sd, pad, get_fmt, NULL, _src);
+   ret = v4l2_subdev_call(sd, pad, get_fmt, NULL, _src);
if (ret) {
v4l2_err(sd, "failed to get src_sd format\n");
goto unreg;
-- 
2.25.1

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

[PATCH AUTOSEL 5.4 246/330] staging:r8188eu: avoid skb_clone for amsdu to msdu conversion

2020-09-17 Thread Sasha Levin 
From: Ivan Safonov 

[ Upstream commit 628cbd971a927abe6388d44320e351c337b331e4 ]

skb clones use same data buffer,
so tail of one skb is corrupted by beginning of next skb.

Signed-off-by: Ivan Safonov 
Link: https://lore.kernel.org/r/20200423191404.12028-1-insafo...@gmail.com
Signed-off-by: Greg Kroah-Hartman 
Signed-off-by: Sasha Levin 
---
 drivers/staging/rtl8188eu/core/rtw_recv.c | 19 ++-
 1 file changed, 6 insertions(+), 13 deletions(-)

diff --git a/drivers/staging/rtl8188eu/core/rtw_recv.c 
b/drivers/staging/rtl8188eu/core/rtw_recv.c
index d4278361e0028..a036ef104198e 100644
--- a/drivers/staging/rtl8188eu/core/rtw_recv.c
+++ b/drivers/staging/rtl8188eu/core/rtw_recv.c
@@ -1525,21 +1525,14 @@ static int amsdu_to_msdu(struct adapter *padapter, 
struct recv_frame *prframe)
 
/* Allocate new skb for releasing to upper layer */
sub_skb = dev_alloc_skb(nSubframe_Length + 12);
-   if (sub_skb) {
-   skb_reserve(sub_skb, 12);
-   skb_put_data(sub_skb, pdata, nSubframe_Length);
-   } else {
-   sub_skb = skb_clone(prframe->pkt, GFP_ATOMIC);
-   if (sub_skb) {
-   sub_skb->data = pdata;
-   sub_skb->len = nSubframe_Length;
-   skb_set_tail_pointer(sub_skb, nSubframe_Length);
-   } else {
-   DBG_88E("skb_clone() Fail!!! , 
nr_subframes=%d\n", nr_subframes);
-   break;
-   }
+   if (!sub_skb) {
+   DBG_88E("dev_alloc_skb() Fail!!! , nr_subframes=%d\n", 
nr_subframes);
+   break;
}
 
+   skb_reserve(sub_skb, 12);
+   skb_put_data(sub_skb, pdata, nSubframe_Length);
+
subframes[nr_subframes++] = sub_skb;
 
if (nr_subframes >= MAX_SUBFRAME_COUNT) {
-- 
2.25.1

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

[PATCH AUTOSEL 5.4 137/330] media: staging/imx: Missing assignment in imx_media_capture_device_register()

2020-09-17 Thread Sasha Levin 
From: Dan Carpenter 

[ Upstream commit ef0ed05dcef8a74178a8b480cce23a377b1de2b8 ]

There was supposed to be a "ret = " assignment here, otherwise the
error handling on the next line won't work.

Fixes: 64b5a49df486 ("[media] media: imx: Add Capture Device Interface")
Signed-off-by: Dan Carpenter 
Reviewed-by: Steve Longerbeam 
Signed-off-by: Hans Verkuil 
Signed-off-by: Mauro Carvalho Chehab 
Signed-off-by: Sasha Levin 
---
 drivers/staging/media/imx/imx-media-capture.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/media/imx/imx-media-capture.c 
b/drivers/staging/media/imx/imx-media-capture.c
index 46576e32581f0..d151cd6d31884 100644
--- a/drivers/staging/media/imx/imx-media-capture.c
+++ b/drivers/staging/media/imx/imx-media-capture.c
@@ -785,7 +785,7 @@ int imx_media_capture_device_register(struct 
imx_media_video_dev *vdev)
/* setup default format */
fmt_src.pad = priv->src_sd_pad;
fmt_src.which = V4L2_SUBDEV_FORMAT_ACTIVE;
-   v4l2_subdev_call(sd, pad, get_fmt, NULL, _src);
+   ret = v4l2_subdev_call(sd, pad, get_fmt, NULL, _src);
if (ret) {
v4l2_err(sd, "failed to get src_sd format\n");
goto unreg;
-- 
2.25.1

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

Re: [PATCH v2] media: cedrus: Propagate OUTPUT resolution to CAPTURE

2020-09-17 Thread Nicolas Dufresne 
Le jeudi 17 septembre 2020 à 20:27 -0400, Nicolas Dufresne a écrit :
> As per spec, the CAPTURE resolution should be automatically set based on
> the OTUPUT resolution. This patch properly propagate width/height to the
> capture when the OUTPUT format is set and override the user provided
> width/height with configured OUTPUT resolution when the CAPTURE fmt is
> updated.
> 
> This also prevents userspace from selecting a CAPTURE resolution that is
> too small, avoiding kernel oops.

Just in case it wasn't obvious, this is fully reproducible oops
whenever you use GStreamer 1.18. Here's a copy of Ondrej report from
today which thankfully allowed me to realized I had never completed
this patch. Pretty much all kernel that includes Cedrus are to be
affect, though is a staging driver on staging API of course.

---

I tried testing cedrus with gstreamer 1.18 and it managed to crash the
kernel on
A64. I used:

  gst-launch-1.0 filesrc location=test.mkv ! matroskademux ! queue !
h264parse ! v4l2slh264dec ! filesink location=aaa.dat

Unable to handle kernel paging request at virtual address
8080808080808088
Mem abort info:
  ESR = 0x9644
  EC = 0x25: DABT (current EL), IL = 32 bits
  SET = 0, FnV = 0
  EA = 0, S1PTW = 0
Data abort info:
  ISV = 0, ISS = 0x0044
  CM = 0, WnR = 1
[8080808080808088] address between user and kernel address ranges
Internal error: Oops: 9644 [#1] SMP
Modules linked in: modem_power hci_uart btrtl bluetooth ecdh_generic
ecc sunxi_cedrus(C) sun8i_ce crypto_engine snd_soc_bt_sco
snd_soc_simple_card snd_soc_simple_card_utils snd_soc_simple_amplifier
sun50i_codec_analog sun8i_codec sun8i_adda_pr_regmap snd_soc_ec25
sun4i_i2s snd_soc_core snd_pcm_dmaengine snd_pcm snd_timer snd
soundcore stk3310 inv_mpu6050_i2c inv_mpu6050 st_magn_i2c
st_sensors_i2c st_magn st_sensors industrialio_triggered_buffer
kfifo_buf regmap_i2c option usb_wwan usbserial anx7688 ohci_platform
ohci_hcd ehci_platform ehci_hcd g_cdc usb_f_acm u_serial usb_f_ecm
u_ether libcomposite sunxi phy_generic musb_hdrc udc_core usbcore
sun8i_rotate v4l2_mem2mem gc2145 ov5640 sun6i_csi videobuf2_dma_contig
v4l2_fwnode videobuf2_memops videobuf2_v4l2 videobuf2_common videodev
mc 8723cs(C) cfg80211 rfkill lima gpu_sched goodix
CPU: 0 PID: 0 Comm: swapper/0 Tainted: G C5.9.0-rc5-
00386-g4fe2ef82bd0b #62
Hardware name: Pine64 PinePhone (1.2) (DT)
pstate: 8085 (Nzcv daIf -PAN -UAO BTYPE=--)
pc : v4l2_m2m_buf_remove+0x44/0x90 [v4l2_mem2mem]
lr : v4l2_m2m_buf_remove+0x18/0x90 [v4l2_mem2mem]
sp : ffc010c8be20
x29: ffc010c8be20 x28: ffc010bb2fc0 
x27: 0060 x26: ffc010935e58 
x25: ffc010c06a5a x24: 0080 
x23: 0005 x22: ffc010c8bf4c 
x21: ff806ba0d088 x20: ff80687d1800 
x19: ff8066c40298 x18:  
x17:  x16:  
x15: 01b66678fd80 x14: 0204 
x13: 0001 x12: 0040 
x11: ff806f0c0248 x10: ff806f0c024a 
x9 : ffc010bbdac8 x8 : ff806f000270 
x7 :  x6 : dead0100 
x5 : dead0122 x4 :  
x3 : 8080808080808080 x2 : 8080808080808080 
x1 : ff80641327a8 x0 : 0080 
Call trace:
 v4l2_m2m_buf_remove+0x44/0x90 [v4l2_mem2mem]
 v4l2_m2m_buf_done_and_job_finish+0x34/0x140 [v4l2_mem2mem]
 cedrus_irq+0x8c/0xc0 [sunxi_cedrus]
 __handle_irq_event_percpu+0x54/0x150
 handle_irq_event+0x4c/0xec
 handle_fasteoi_irq+0xbc/0x1c0
 __handle_domain_irq+0x78/0xdc
 gic_handle_irq+0x50/0xa0
 el1_irq+0xb8/0x140
 arch_cpu_idle+0x10/0x14
 cpu_startup_entry+0x24/0x60
 rest_init+0xb0/0xbc
 arch_call_rest_init+0xc/0x14
 start_kernel+0x690/0x6b0
Code: f2fbd5a6 f2fbd5a5 5284 a9400823 (f9000462) 
---[ end trace 88233b9a76cdb261 ]---
Kernel panic - not syncing: Fatal exception in interrupt

> 
> Signed-off-by: Nicolas Dufresne 
> Reviewed-by: Ezequiel Garcia 
> Acked-by: Paul Kocialkowski 
> Tested-by: Ondrej Jirman 
> ---
>  .../staging/media/sunxi/cedrus/cedrus_video.c | 29 +--
>  1 file changed, 27 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_video.c 
> b/drivers/staging/media/sunxi/cedrus/cedrus_video.c
> index 16d82309e7b6..667b86dde1ee 100644
> --- a/drivers/staging/media/sunxi/cedrus/cedrus_video.c
> +++ b/drivers/staging/media/sunxi/cedrus/cedrus_video.c
> @@ -247,6 +247,8 @@ static int cedrus_try_fmt_vid_cap(struct file *file, void 
> *priv,
>   return -EINVAL;
>  
>   pix_fmt->pixelformat = fmt->pixelformat;
> + pix_fmt->width = ctx->src_fmt.width;
> + pix_fmt->height = ctx->src_fmt.height;
>   cedrus_prepare_format(pix_fmt);
>  
>   return 0;
> @@ -296,10 +298,30 @@ static int cedrus_s_fmt_vid_out(struct file *file, void 
> *priv,
>  {
>   struct cedrus_ctx *ctx = cedrus_file2ctx(file);
>   struct vb2_queue *vq;
> + struct vb2_queue *peer_vq;
>   int ret;
>  
> + ret = cedrus_try_fmt_vid_out(file, priv, f);

[PATCH v2] media: cedrus: Propagate OUTPUT resolution to CAPTURE

2020-09-17 Thread Nicolas Dufresne 
As per spec, the CAPTURE resolution should be automatically set based on
the OTUPUT resolution. This patch properly propagate width/height to the
capture when the OUTPUT format is set and override the user provided
width/height with configured OUTPUT resolution when the CAPTURE fmt is
updated.

This also prevents userspace from selecting a CAPTURE resolution that is
too small, avoiding kernel oops.

Signed-off-by: Nicolas Dufresne 
Reviewed-by: Ezequiel Garcia 
Acked-by: Paul Kocialkowski 
Tested-by: Ondrej Jirman 
---
 .../staging/media/sunxi/cedrus/cedrus_video.c | 29 +--
 1 file changed, 27 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_video.c 
b/drivers/staging/media/sunxi/cedrus/cedrus_video.c
index 16d82309e7b6..667b86dde1ee 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus_video.c
+++ b/drivers/staging/media/sunxi/cedrus/cedrus_video.c
@@ -247,6 +247,8 @@ static int cedrus_try_fmt_vid_cap(struct file *file, void 
*priv,
return -EINVAL;
 
pix_fmt->pixelformat = fmt->pixelformat;
+   pix_fmt->width = ctx->src_fmt.width;
+   pix_fmt->height = ctx->src_fmt.height;
cedrus_prepare_format(pix_fmt);
 
return 0;
@@ -296,10 +298,30 @@ static int cedrus_s_fmt_vid_out(struct file *file, void 
*priv,
 {
struct cedrus_ctx *ctx = cedrus_file2ctx(file);
struct vb2_queue *vq;
+   struct vb2_queue *peer_vq;
int ret;
 
+   ret = cedrus_try_fmt_vid_out(file, priv, f);
+   if (ret)
+   return ret;
+
vq = v4l2_m2m_get_vq(ctx->fh.m2m_ctx, f->type);
-   if (vb2_is_busy(vq))
+   /*
+* In order to support dynamic resolution change,
+* the decoder admits a resolution change, as long
+* as the pixelformat remains. Can't be done if streaming.
+*/
+   if (vb2_is_streaming(vq) || (vb2_is_busy(vq) &&
+   f->fmt.pix.pixelformat != ctx->src_fmt.pixelformat))
+   return -EBUSY;
+   /*
+* Since format change on the OUTPUT queue will reset
+* the CAPTURE queue, we can't allow doing so
+* when the CAPTURE queue has buffers allocated.
+*/
+   peer_vq = v4l2_m2m_get_vq(ctx->fh.m2m_ctx,
+ V4L2_BUF_TYPE_VIDEO_CAPTURE);
+   if (vb2_is_busy(peer_vq))
return -EBUSY;
 
ret = cedrus_try_fmt_vid_out(file, priv, f);
@@ -319,11 +341,14 @@ static int cedrus_s_fmt_vid_out(struct file *file, void 
*priv,
break;
}
 
-   /* Propagate colorspace information to capture. */
+   /* Propagate format information to capture. */
ctx->dst_fmt.colorspace = f->fmt.pix.colorspace;
ctx->dst_fmt.xfer_func = f->fmt.pix.xfer_func;
ctx->dst_fmt.ycbcr_enc = f->fmt.pix.ycbcr_enc;
ctx->dst_fmt.quantization = f->fmt.pix.quantization;
+   ctx->dst_fmt.width = ctx->src_fmt.width;
+   ctx->dst_fmt.height = ctx->src_fmt.height;
+   cedrus_prepare_format(>dst_fmt);
 
return 0;
 }
-- 
2.26.2

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

FOESCO Informa

2020-09-17 Thread FOESCO 
Buenos días


Soy Alex Pons, director de FOESCO (Formación Estatal Continua).

Llegadas estas fechas y como cada año, recordamos a todas las empresas 
Españolas su derecho a consumir el Crédito de Formación del que disponen para 
la formación de sus empleados, antes de su caducidad a final de año.


Actualmente se encuentra abierto el plazo de inscripción para la Convocatoria 
SEPTIEMBRE 2020 de Cursos Bonificables con cargo al Crédito de Formación 2020.


Deseáis que os mandemos la información?


Quedamos a la espera de vuestra respuesta.


Saludos cordiales.


Alex Pons
Director FOESCO.

FOESCO Formación Estatal Continua.
Entidad Organizadora: B200592AA
www.foesco.com
e-mail: cur...@foesco.net
Tel: 910 323 794
(Horario de 9h a 15h y de 17h a 20h de Lunes a Viernes)

FOESCO ofrece formación a empresas y trabajadores en activo a través de cursos 
bonificados por la Fundación Estatal para la Formación en el Empleo (antiguo 
FORCEM) que gestiona las acciones formativas de FORMACIÓN CONTINUA para 
trabajadores y se rige por la ley 30/2015 de 9 de Septiembre.

Antes de imprimir este e-mail piense bien si es necesario hacerlo. Before 
printing this e-mail please think twice if you really need it. FOESCO Tfno: 910 
382 880 Email: cur...@foesco.com. La información transmitida en este mensaje 
está dirigida solamente a las personas o entidades que figuran en el 
encabezamiento y contiene información confidencial, por lo que, si usted lo 
recibiera por error, por favor destrúyalo sin copiarlo, usarlo ni distribuirlo, 
comunicándolo inmediatamente al emisor del mensaje. De conformidad con lo 
dispuesto en el Reglamento Europeo del 2016/679, del 27 de Abril de 2016, 
FOESCO le informa que los datos por usted suministrados serán tratados con las 
medidas de seguridad conformes a la normativa vigente que se requiere. Dichos 
datos serán empleados con fines de gestión. Para el ejercicio de sus derechos 
de transparencia, información, acceso, rectificación, supresión o derecho al 
olvido, limitación del tratamiento , portabilidad de datos y oposición de sus 
datos de carácter personal deberá dirigirse a la dirección del Responsable del 
tratamiento a C/ LAGUNA DEL MARQUESADO Nº10, 28021, MADRID, "PULSANDO AQUI" 
 y "ENVIAR" o a traves de la 
dirección de correo electrónico: ba...@foesco.com 

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

Offer for humanitarian work in your country

2020-09-17 Thread Jane Lilly 



HELLO, 
My name is Mrs. Jane from America, I read about you from a reliable web site 
and i will love to employ you into humanitarian work in your country, I'm ready 
to donate some money for you to carry out the work in your country. Please 
reply with your private email address so that i will give you further details 
and tell you about myself.

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

Re: [RFC PATCH] Add bridge driver to connect sensors to CIO2 device via software nodes on ACPI platforms

2020-09-17 Thread Daniel Scally 
On 17/09/2020 15:14, Andy Shevchenko wrote:
> On Thu, Sep 17, 2020 at 4:53 PM Dan Scally  wrote:
>> Hi Andy, thanks for input (as always)
> You're welcome! I'm really impressed by your activity in this area.
Thanks - it's pretty fun so far
>>> Ah, I think you misinterpreted the meaning of above. The above is a switch 
>>> how
>>> camera device appears either as PCI or an ACPI. So, it effectively means you
>>> should *not* have any relation for this HID until you find a platform where 
>>> the
>>> device is for real enumerated via ACPI.
>>>
>> Ah, ok. So that was never going to work. Thanks. That does raise another
>> question; we have had some testers report failure, which turns out to be
>> because on their platforms the definition of their cameras in ACPI is
>> never translated into an i2c_client so the cio2-bridge doesn't bind.
>> Those have a similar conditional in the _STA method, see CAM1 in this
>> DSDT for example:
>> https://raw.githubusercontent.com/linux-surface/acpidumps/master/surface_go/dsdt.dsl.
>> Is there anything we can do to enable those cameras to be discovered too?
> It means that this
Is the rest of this comment missing?
>> +#define PROPERTY_ENTRY_NULL   \
>> +((const struct property_entry) { })
> Alignment. Same appears to apply to other macros (please indent).
 Yep
>> +#define SOFTWARE_NODE_NULL\
>> +((const struct software_node) { })
>>> Why?!
>>>
>> It felt ugly to have the other definitions be macros and not this one,
>> but I can change it.
> My point is that those macros are simply redundant. The point is to
> have a terminator record (all 0:s in the last entry of an array) which
> is usually being achieved by allocating memory with kcalloc() which
> does implicitly this for you.
Ah I see. TIL - thanks, I'll make that change too.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[driver-core:driver-core-testing] BUILD SUCCESS b85300173d027131ced9a654c506785f15cfdd6f

2020-09-17 Thread kernel test robot 
tree/branch: 
https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/driver-core.git  
driver-core-testing
branch HEAD: b85300173d027131ced9a654c506785f15cfdd6f  driver core: force NOIO 
allocations during unplug

elapsed time: 724m

configs tested: 113
configs skipped: 2

The following configs have been built successfully.
More configs may be tested in the coming days.

gcc tested configs:
arm defconfig
arm64allyesconfig
arm64   defconfig
arm  allyesconfig
arm  allmodconfig
mipsomega2p_defconfig
arm lubbock_defconfig
arm   spitz_defconfig
powerpcwarp_defconfig
shdreamcast_defconfig
parisc   allyesconfig
m68k  hp300_defconfig
powerpc  mgcoge_defconfig
powerpc mpc832x_mds_defconfig
armu300_defconfig
m68k allyesconfig
s390  debug_defconfig
sh  polaris_defconfig
powerpc asp8347_defconfig
armmini2440_defconfig
mipsmaltaup_xpa_defconfig
armmvebu_v7_defconfig
powerpc   eiger_defconfig
powerpc tqm8548_defconfig
sh ecovec24_defconfig
arm   imx_v4_v5_defconfig
m68k alldefconfig
arm vf610m4_defconfig
arm  collie_defconfig
arm   efm32_defconfig
arm mxs_defconfig
archsdk_defconfig
arm   u8500_defconfig
arm  integrator_defconfig
mips decstation_defconfig
powerpc   lite5200b_defconfig
sh   sh2007_defconfig
shsh7757lcr_defconfig
powerpc  iss476-smp_defconfig
arm   omap2plus_defconfig
sh   se7712_defconfig
powerpc  katmai_defconfig
arm lpc18xx_defconfig
mips   rs90_defconfig
armvexpress_defconfig
pariscgeneric-32bit_defconfig
riscv allnoconfig
ia64 allmodconfig
ia64defconfig
ia64 allyesconfig
m68k allmodconfig
m68kdefconfig
nios2   defconfig
arc  allyesconfig
nds32 allnoconfig
c6x  allyesconfig
nds32   defconfig
nios2allyesconfig
cskydefconfig
alpha   defconfig
alphaallyesconfig
xtensa   allyesconfig
h8300allyesconfig
arc defconfig
sh   allmodconfig
parisc  defconfig
s390 allyesconfig
s390defconfig
i386 allyesconfig
sparcallyesconfig
sparc   defconfig
i386defconfig
mips allyesconfig
mips allmodconfig
powerpc  allyesconfig
powerpc  allmodconfig
powerpc   allnoconfig
i386 randconfig-a004-20200917
i386 randconfig-a006-20200917
i386 randconfig-a003-20200917
i386 randconfig-a001-20200917
i386 randconfig-a002-20200917
i386 randconfig-a005-20200917
x86_64   randconfig-a014-20200917
x86_64   randconfig-a011-20200917
x86_64   randconfig-a016-20200917
x86_64   randconfig-a012-20200917
x86_64   randconfig-a015-20200917
x86_64   randconfig-a013-20200917
i386 randconfig-a015-20200917
i386 randconfig-a014-20200917
i386 randconfig-a011-20200917
i386 randconfig-a013-20200917
i386 randconfig-a016-20200917
i386 randconfig-a012-20200917
riscvnommu_k210_defconfig
riscvallyesconfig
riscvnommu_virt_defconfig
riscv   defconfig
riscv  rv32_defconfig

Re: [PATCH] media: cedrus: Propagate OUTPUT resolution to CAPTURE

2020-09-17 Thread Nicolas Dufresne 
Le jeudi 17 septembre 2020 à 12:39 +0200, Hans Verkuil a écrit :
> On 14/05/2020 17:39, Nicolas Dufresne wrote:
> > As per spec, the CAPTURE resolution should be automatically set based on
> > the OTUPUT resolution. This patch properly propagate width/height to the
> > capture when the OUTPUT format is set and override the user provided
> > width/height with configured OUTPUT resolution when the CAPTURE fmt is
> > updated.
> > 
> > This also prevents userspace from selecting a CAPTURE resolution that is
> > too small, avoiding unwanted page faults.
> > 
> > Signed-off-by: Nicolas Dufresne 
> > ---
> >  drivers/staging/media/sunxi/cedrus/cedrus_video.c | 7 ++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_video.c 
> > b/drivers/staging/media/sunxi/cedrus/cedrus_video.c
> > index 16d82309e7b6..a6d6b15adc2e 100644
> > --- a/drivers/staging/media/sunxi/cedrus/cedrus_video.c
> > +++ b/drivers/staging/media/sunxi/cedrus/cedrus_video.c
> > @@ -247,6 +247,8 @@ static int cedrus_try_fmt_vid_cap(struct file *file, 
> > void *priv,
> > return -EINVAL;
> >  
> > pix_fmt->pixelformat = fmt->pixelformat;
> > +   pix_fmt->width = ctx->src_fmt.width;
> > +   pix_fmt->height = ctx->src_fmt.height;
> > cedrus_prepare_format(pix_fmt);
> >  
> > return 0;
> > @@ -319,11 +321,14 @@ static int cedrus_s_fmt_vid_out(struct file *file, 
> > void *priv,
> > break;
> > }
> >  
> > -   /* Propagate colorspace information to capture. */
> > +   /* Propagate format information to capture. */
> > ctx->dst_fmt.colorspace = f->fmt.pix.colorspace;
> > ctx->dst_fmt.xfer_func = f->fmt.pix.xfer_func;
> > ctx->dst_fmt.ycbcr_enc = f->fmt.pix.ycbcr_enc;
> > ctx->dst_fmt.quantization = f->fmt.pix.quantization;
> > +   ctx->dst_fmt.width = ctx->src_fmt.width;
> > +   ctx->dst_fmt.height = ctx->src_fmt.height;
> 
> You can only do this if the capture queue isn't busy.
> 
> See also hantro_reset_raw_fmt() where it does that check.
> 
> So this needs a v2.

I missed this reply, wasn't expecting a negative third review. I'll
copy and paste from there. So basically:

 - EBUSY if peer vq is busy
 - EBUSY if pixel format is changed while vq is busy
 - EBUSY if streaming

Obviously, I don't have userspace to exercise any of these cases, but
it seems to make sense. I remembered about this patch today as someone
reported the kernel crash we get without this patch:

---

I tried testing cedrus with gstreamer 0.18 and it managed to crash the
kernel on
A64. I used:

  gst-launch-1.0 filesrc location=test.mkv ! matroskademux ! queue !
h264parse ! v4l2slh264dec ! filesink location=aaa.dat

Unable to handle kernel paging request at virtual address
8080808080808088
Mem abort info:
  ESR = 0x9644
  EC = 0x25: DABT (current EL), IL = 32 bits
  SET = 0, FnV = 0
  EA = 0, S1PTW = 0
Data abort info:
  ISV = 0, ISS = 0x0044
  CM = 0, WnR = 1
[8080808080808088] address between user and kernel address ranges
Internal error: Oops: 9644 [#1] SMP
Modules linked in: modem_power hci_uart btrtl bluetooth ecdh_generic
ecc sunxi_cedrus(C) sun8i_ce crypto_engine snd_soc_bt_sco
snd_soc_simple_card snd_soc_simple_card_utils snd_soc_simple_amplifier
sun50i_codec_analog sun8i_codec sun8i_adda_pr_regmap snd_soc_ec25
sun4i_i2s snd_soc_core snd_pcm_dmaengine snd_pcm snd_timer snd
soundcore stk3310 inv_mpu6050_i2c inv_mpu6050 st_magn_i2c
st_sensors_i2c st_magn st_sensors industrialio_triggered_buffer
kfifo_buf regmap_i2c option usb_wwan usbserial anx7688 ohci_platform
ohci_hcd ehci_platform ehci_hcd g_cdc usb_f_acm u_serial usb_f_ecm
u_ether libcomposite sunxi phy_generic musb_hdrc udc_core usbcore
sun8i_rotate v4l2_mem2mem gc2145 ov5640 sun6i_csi videobuf2_dma_contig
v4l2_fwnode videobuf2_memops videobuf2_v4l2 videobuf2_common videodev
mc 8723cs(C) cfg80211 rfkill lima gpu_sched goodix
CPU: 0 PID: 0 Comm: swapper/0 Tainted: G C5.9.0-rc5-
00386-g4fe2ef82bd0b #62
Hardware name: Pine64 PinePhone (1.2) (DT)
pstate: 8085 (Nzcv daIf -PAN -UAO BTYPE=--)
pc : v4l2_m2m_buf_remove+0x44/0x90 [v4l2_mem2mem]
lr : v4l2_m2m_buf_remove+0x18/0x90 [v4l2_mem2mem]
sp : ffc010c8be20
x29: ffc010c8be20 x28: ffc010bb2fc0 
x27: 0060 x26: ffc010935e58 
x25: ffc010c06a5a x24: 0080 
x23: 0005 x22: ffc010c8bf4c 
x21: ff806ba0d088 x20: ff80687d1800 
x19: ff8066c40298 x18:  
x17:  x16:  
x15: 01b66678fd80 x14: 0204 
x13: 0001 x12: 0040 
x11: ff806f0c0248 x10: ff806f0c024a 
x9 : ffc010bbdac8 x8 : ff806f000270 
x7 :  x6 : dead0100 
x5 : dead0122 x4 :  
x3 : 8080808080808080 x2 : 8080808080808080 
x1 : ff80641327a8 x0 : 0080 
Call trace:
 v4l2_m2m_buf_remove+0x44/0x90 [v4l2_mem2mem]

Re: [PATCH 0/5] ARM: dts: sun8i: r40: Enable video decoder

2020-09-17 Thread Maxime Ripard 
On Thu, Sep 17, 2020 at 10:33:39AM +0200, Hans Verkuil wrote:
> Hi Maxime,
> 
> On 27/08/2020 17:19, Maxime Ripard wrote:
> > On Tue, Aug 25, 2020 at 07:35:18PM +0200, Jernej Skrabec wrote:
> >> Allwinner R40 SoC contains video engine very similar to that in A33.
> >>
> >> First two patches add system controller nodes and the rest of them
> >> add support for Cedrus VPU.
> >>
> >> Please take a look.
> > 
> > Applied all 5 patches, thanks
> 
> Just to confirm: you've taken patches 3 and 4 as well? If so, then I
> can mark them as done in patchwork.

Uh... Yeah, I did, but they were definitely not mine to take... I'm
sorry, I'll drop them and you can merge totally merge them :)

Maxime


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

Re: [RFC PATCH] Add bridge driver to connect sensors to CIO2 device via software nodes on ACPI platforms

2020-09-17 Thread Andy Shevchenko 
On Thu, Sep 17, 2020 at 02:36:22PM +0100, Dan Scally wrote:
> On 17/09/2020 13:45, Andy Shevchenko wrote:
> > On Thu, Sep 17, 2020 at 11:52:28AM +0100, Dan Scally wrote:
> >> On 17/09/2020 11:33, Sakari Ailus wrote:
> > I will do better review for next version, assuming you will Cc reviewers and
> > TWIMC people. Below is like small part of comments I may give to the code.
> TWIMC?

Missed this. To Whom It May Concern.

-- 
With Best Regards,
Andy Shevchenko


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

Re: [RFC PATCH] Add bridge driver to connect sensors to CIO2 device via software nodes on ACPI platforms

2020-09-17 Thread Andy Shevchenko 
On Thu, Sep 17, 2020 at 5:19 PM Kieran Bingham
 wrote:
> On 17/09/2020 15:08, Andy Shevchenko wrote:

...

> Ayee, ok so we have 'half' the driver for IPU3 out of staging.

Correct. And your below analysis is correct.

> From my understanding, the IPU3 consists of two components, the CIO2
> (CSI2 capture), and the IMGU (the ISP).
>
> - drivers/media/pci/intel/ipu3
>
> This is indeed the CIO2 component (config VIDEO_IPU3_CIO2), and that is
> the part that this bridge relates to, so in fact this cio2-bridge should
> probably go there indeed. No need to go through staging.
>
> The files remaining at:
>
> - drivers/staging/media/ipu3
>
> are in fact also for the IPU3 but the ISP component (VIDEO_IPU3_IMGU).
>
> I'm sorry for the confusion, I knew that the ISP was still in staging, I
> hadn't realised the CSI2 receiver (CIO2) was not.
>
> >> Hopefully with more users of the IPU3 brought in by this cio2-bridge,
> >> that will help gather momentum to get the IPU3 developments required
> >> completed and moved into drivers/media.

-- 
With Best Regards,
Andy Shevchenko
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [RFC PATCH] Add bridge driver to connect sensors to CIO2 device via software nodes on ACPI platforms

2020-09-17 Thread Kieran Bingham 
Hi Andy,

On 17/09/2020 15:08, Andy Shevchenko wrote:
> On Thu, Sep 17, 2020 at 4:31 PM Kieran Bingham
>  wrote:
>> On 17/09/2020 10:47, Dan Scally wrote:
>>> On 17/09/2020 08:53, Greg KH wrote:
 On Wed, Sep 16, 2020 at 10:36:18PM +0100, Daniel Scally wrote:
> 
>  drivers/staging/media/ipu3/Kconfig   |  15 +
>  drivers/staging/media/ipu3/Makefile  |   1 +
>  drivers/staging/media/ipu3/cio2-bridge.c | 448 +++
 Why does this have to be in drivers/staging/ at all?  Why not spend the
 time to fix it up properly and get it merged correctly?  It's a very
 small driver, and should be smaller, so it should not be a lot of work
 to do.  And it would be faster to do that than to take it through
 staging...
>>> I was just under the impression that that was the process to be honest,
>>> if that's not right I'll just move it directly to drivers/media/ipu3
>>
>> The IPU3 driver is still in staging (unless I've missed something), so I
>> think this cio2-bridge should stay with it.
> 
> You missed something.
> https://elixir.bootlin.com/linux/v5.9-rc5/source/drivers/media/pci/intel
> 
> IPU3 from Freescale (IIRC) is a different story.

Ayee, ok so we have 'half' the driver for IPU3 out of staging.

>From my understanding, the IPU3 consists of two components, the CIO2
(CSI2 capture), and the IMGU (the ISP).

- drivers/media/pci/intel/ipu3

This is indeed the CIO2 component (config VIDEO_IPU3_CIO2), and that is
the part that this bridge relates to, so in fact this cio2-bridge should
probably go there indeed. No need to go through staging.

The files remaining at:

- drivers/staging/media/ipu3

are in fact also for the IPU3 but the ISP component (VIDEO_IPU3_IMGU).

I'm sorry for the confusion, I knew that the ISP was still in staging, I
hadn't realised the CSI2 receiver (CIO2) was not.

>> Hopefully with more users of the IPU3 brought in by this cio2-bridge,
>> that will help gather momentum to get the IPU3 developments required
>> completed and moved into drivers/media.

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

Re: [RFC PATCH] Add bridge driver to connect sensors to CIO2 device via software nodes on ACPI platforms

2020-09-17 Thread Andy Shevchenko 
On Thu, Sep 17, 2020 at 4:53 PM Dan Scally  wrote:
>
> Hi Andy, thanks for input (as always)

You're welcome! I'm really impressed by your activity in this area.

> On 17/09/2020 13:45, Andy Shevchenko wrote:
> > On Thu, Sep 17, 2020 at 11:52:28AM +0100, Dan Scally wrote:
> >> On 17/09/2020 11:33, Sakari Ailus wrote:

To the point of placement, I think this should go under
drivers/platform/x86 (Adding Hans and Mark, who can express their
opinions).

...

> > Ah, I think you misinterpreted the meaning of above. The above is a switch 
> > how
> > camera device appears either as PCI or an ACPI. So, it effectively means you
> > should *not* have any relation for this HID until you find a platform where 
> > the
> > device is for real enumerated via ACPI.
> >
> Ah, ok. So that was never going to work. Thanks. That does raise another
> question; we have had some testers report failure, which turns out to be
> because on their platforms the definition of their cameras in ACPI is
> never translated into an i2c_client so the cio2-bridge doesn't bind.
> Those have a similar conditional in the _STA method, see CAM1 in this
> DSDT for example:
> https://raw.githubusercontent.com/linux-surface/acpidumps/master/surface_go/dsdt.dsl.
> Is there anything we can do to enable those cameras to be discovered too?

It means that this

...

>  +#define PROPERTY_ENTRY_NULL   \
>  +((const struct property_entry) { })
> >>> Alignment. Same appears to apply to other macros (please indent).
> >> Yep
>  +#define SOFTWARE_NODE_NULL\
>  +((const struct software_node) { })
> > Why?!
> >
> It felt ugly to have the other definitions be macros and not this one,
> but I can change it.

My point is that those macros are simply redundant. The point is to
have a terminator record (all 0:s in the last entry of an array) which
is usually being achieved by allocating memory with kcalloc() which
does implicitly this for you.

-- 
With Best Regards,
Andy Shevchenko
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [RFC PATCH] Add bridge driver to connect sensors to CIO2 device via software nodes on ACPI platforms

2020-09-17 Thread Andy Shevchenko 
On Thu, Sep 17, 2020 at 4:31 PM Kieran Bingham
 wrote:
> On 17/09/2020 10:47, Dan Scally wrote:
> > On 17/09/2020 08:53, Greg KH wrote:
> >> On Wed, Sep 16, 2020 at 10:36:18PM +0100, Daniel Scally wrote:

> >>>  drivers/staging/media/ipu3/Kconfig   |  15 +
> >>>  drivers/staging/media/ipu3/Makefile  |   1 +
> >>>  drivers/staging/media/ipu3/cio2-bridge.c | 448 +++
> >> Why does this have to be in drivers/staging/ at all?  Why not spend the
> >> time to fix it up properly and get it merged correctly?  It's a very
> >> small driver, and should be smaller, so it should not be a lot of work
> >> to do.  And it would be faster to do that than to take it through
> >> staging...
> > I was just under the impression that that was the process to be honest,
> > if that's not right I'll just move it directly to drivers/media/ipu3
>
> The IPU3 driver is still in staging (unless I've missed something), so I
> think this cio2-bridge should stay with it.

You missed something.
https://elixir.bootlin.com/linux/v5.9-rc5/source/drivers/media/pci/intel

IPU3 from Freescale (IIRC) is a different story.

> Hopefully with more users of the IPU3 brought in by this cio2-bridge,
> that will help gather momentum to get the IPU3 developments required
> completed and moved into drivers/media.

-- 
With Best Regards,
Andy Shevchenko
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [RFC PATCH] Add bridge driver to connect sensors to CIO2 device via software nodes on ACPI platforms

2020-09-17 Thread Dan Scally 
Hi Andy, thanks for input (as always)

On 17/09/2020 13:45, Andy Shevchenko wrote:
> On Thu, Sep 17, 2020 at 11:52:28AM +0100, Dan Scally wrote:
>> On 17/09/2020 11:33, Sakari Ailus wrote:
> I will do better review for next version, assuming you will Cc reviewers and
> TWIMC people. Below is like small part of comments I may give to the code.
TWIMC?
>>> The ones I know require PMIC control done in software (not even
>>> sensors are accessible without that).
>> So far we've just been getting the sensor drivers themselves to toggle
>> the gpio pins that turn the PMIC on (those pins are listed against the
>> PMIC's _CRS, and we've been finding those by evaluating the sensor's
>> _DEP) - once that's done the cameras show up on i2c and,with the bridge
>> driver installed, you can use libcamera to take photos.
> Do I understand correctly that you are able to get pictures from the camera
> hardware?

Yes, using the libcamera project's qcam program. They're poor quality at
the moment, because there's no auto-white-balance / exposure controls in
the ipu3 pipeline yet, but we can take images. Example:


https://user-images.githubusercontent.com/4592235/91197920-d1d41500-e6f3-11ea-8207-1c27cf24dd45.png


A bunch of folks have managed it so far on a couple different platforms
(Surface Book 1, Surface Pro something, an Acer A12 and a Lenovo Miix-510)

 I wanted to raise this as an RFC as although I don't think it's ready for
 integration it has some things that I'd like feedback on, in particular the
 method I chose to make the module be auto-inserted. A more ideal method 
 would
 have been to have the driver be an ACPI driver for the INT343E device, but 
 each
>>> What do you think this device does represent? Devices whose status is
>>> always zero may exist in the table even if they would not be actually
>>> present.
>>>
>>> CIO2 is a PCI device and it has no ACPI (or PNP) ID, or at least should not
>>> have one.
>> This is the ACPI entry I mean:
>>
>> Device (CIO2)
>> {
>> Method (_STA, 0, NotSerialized)  // _STA: Status
>> {
>> If ((CIOE == One))
>> {
>> Return (0x0F)
>> }
>> Else
>> {
>> Return (Zero)
>> }
>> }
>>
>> Name (_HID, "INT343E")  // _HID: Hardware ID
>> Method (_CRS, 0, NotSerialized)  // _CRS: Current Resource Settings
>> {
>> Name (CBUF, ResourceTemplate ()
>> {
>> Interrupt (ResourceConsumer, Level, ActiveLow, Shared, ,, _Y15)
>> {
>> 0x0010,
>> }
>> Memory32Fixed (ReadWrite,
>> 0xFE40, // Address Base
>> 0x0001, // Address Length
>> )
>> })
>> CreateDWordField (CBUF, \_SB.PCI0.CIO2._CRS._Y15._INT, CIOV)  // 
>> _INT: Interrupts
>> CIOV = CIOI /* \CIOI */
>> Return (CBUF) /* \_SB_.PCI0.CIO2._CRS.CBUF */
>> }
>> }
> Ah, I think you misinterpreted the meaning of above. The above is a switch how
> camera device appears either as PCI or an ACPI. So, it effectively means you
> should *not* have any relation for this HID until you find a platform where 
> the
> device is for real enumerated via ACPI.
>
Ah, ok. So that was never going to work. Thanks. That does raise another
question; we have had some testers report failure, which turns out to be
because on their platforms the definition of their cameras in ACPI is
never translated into an i2c_client so the cio2-bridge doesn't bind.
Those have a similar conditional in the _STA method, see CAM1 in this
DSDT for example:
https://raw.githubusercontent.com/linux-surface/acpidumps/master/surface_go/dsdt.dsl.
Is there anything we can do to enable those cameras to be discovered too?

 +static int cio2_probe_can_progress(struct pci_dev *pci_dev)
 +{
 +  void *sensor;
> Why void?
> Besides the fact that castings from or to void * are implicit in C, the proper
> use of list API should have pretty well defined type of lvalue.
>
Yeah, I misunderstood how this worked - after greg pointed out I was
doing it wrong I read the code a bit better and got it working assigning
to a struct device *sensor; - TIL.
 +  if (!IS_ENABLED(CONFIG_ACPI)) {
 +  r = cio2_parse_firmware(cio2);
 +  if (r)
 +  goto fail_clean_notifier;
 +  }
> How comes?
Me misunderstanding again; it will be removed.
>
 \ No newline at end of file
> ???
>
> Be sure you are using good editor.
>
Yeah haven't managed to track down what's causing this yet. Visual
Studio Code maybe.
 +#define PROPERTY_ENTRY_NULL   \
 +((const struct property_entry) { })
>>> Alignment. Same appears to apply to other macros (please indent).
>> Yep
 +#define SOFTWARE_NODE_NULL\
 +((const struct software_node) { })
> Why?!
>
It felt ugly to have the other definitions be macros and not this one,
but I

Re: [RFC PATCH] Add bridge driver to connect sensors to CIO2 device via software nodes on ACPI platforms

2020-09-17 Thread Kieran Bingham 
Hi Dan, Greg,

On 17/09/2020 10:47, Dan Scally wrote:
> Hi Greg - thanks for the comments, appreciate it (sorry there's so many,
> I'm new to both C and kernel work)
> 
> On 17/09/2020 08:53, Greg KH wrote:
>> On Wed, Sep 16, 2020 at 10:36:18PM +0100, Daniel Scally wrote:
>>>  MAINTAINERS  |   6 +
>>>  drivers/media/pci/intel/ipu3/ipu3-cio2.c |  67 +++-
>> staging drivers should be self-contained, and not modify stuff outside
>> of drivers/staging/
>>
>>>  drivers/staging/media/ipu3/Kconfig   |  15 +
>>>  drivers/staging/media/ipu3/Makefile  |   1 +
>>>  drivers/staging/media/ipu3/cio2-bridge.c | 448 +++
>> Why does this have to be in drivers/staging/ at all?  Why not spend the
>> time to fix it up properly and get it merged correctly?  It's a very
>> small driver, and should be smaller, so it should not be a lot of work
>> to do.  And it would be faster to do that than to take it through
>> staging...
> I was just under the impression that that was the process to be honest,
> if that's not right I'll just move it directly to drivers/media/ipu3

The IPU3 driver is still in staging (unless I've missed something), so I
think this cio2-bridge should stay with it.

Hopefully with more users of the IPU3 brought in by this cio2-bridge,
that will help gather momentum to get the IPU3 developments required
completed and moved into drivers/media.



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

Re: [RFC PATCH] Add bridge driver to connect sensors to CIO2 device via software nodes on ACPI platforms

2020-09-17 Thread Dan Carpenter 
On Thu, Sep 17, 2020 at 03:25:29PM +0300, Andy Shevchenko wrote:
> On Thu, Sep 17, 2020 at 01:49:41PM +0300, Dan Carpenter wrote:
> > On Thu, Sep 17, 2020 at 01:33:43PM +0300, Sakari Ailus wrote:
> 
> > > > +   int i, ret;
> > > 
> > > unsigned int i
> > > 
> > 
> > Why?
> > 
> > For list iterators then "int i;" is best...  For sizes then unsigned is
> > sometimes best.  Or if it's part of the hardware spec or network spec
> > unsigned is best.  Otherwise unsigned variables cause a ton of bugs.
> > They're not as intuitive as signed variables.  Imagine if there is an
> > error in this loop and you want to unwind.  With a signed variable you
> > can do:
> > 
> > while (--i >= 0)
> > cleanup([i]);
> 
> Ha-ha. It's actually a counter argument to your stuff because above is the 
> same as
> 
>   while (i--)
>   cleanup([i]);
> 
> with pretty much unsigned int i.

With vanilla "int i;" you can't go wrong because both styles work as
expected.  I was just giving examples of real life bugs that I have seen
involving unsigned iterators.

54313503f9a3 ("drm/amdgpu: signedness bug in amdgpu_cs_parser_init()")

Here are a couple more bugs involving unsigned iterators...

d72cf01f410a ("drm/mipi-dbi: fix a loop in debugfs code")
ce6c1cd2c324 ("pinctrl: nsp-gpio: forever loop in nsp_gpio_get_strength()")

I've change a lot of variables unsigned as well.  For min_t() then it
should *always* be an unsigned type.

It's pretty rare to iterate over 2 billion times in the kernel, but
there are times when you might want to do that.  Normally you would
want to declare the iterator as an unsigned ong in that case.  But most
of the time iterators should just be "int i;" to prevent bugs.

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

Re: [RFC PATCH] Add bridge driver to connect sensors to CIO2 device via software nodes on ACPI platforms

2020-09-17 Thread Andy Shevchenko 
On Thu, Sep 17, 2020 at 11:52:28AM +0100, Dan Scally wrote:
> On 17/09/2020 11:33, Sakari Ailus wrote:

I will do better review for next version, assuming you will Cc reviewers and
TWIMC people. Below is like small part of comments I may give to the code.

...

> > The ones I know require PMIC control done in software (not even
> > sensors are accessible without that).
> So far we've just been getting the sensor drivers themselves to toggle
> the gpio pins that turn the PMIC on (those pins are listed against the
> PMIC's _CRS, and we've been finding those by evaluating the sensor's
> _DEP) - once that's done the cameras show up on i2c and,with the bridge
> driver installed, you can use libcamera to take photos.

Do I understand correctly that you are able to get pictures from the camera
hardware?

...

> > a module and not enlarge everyone's kernel, and the initialisation would at
> > the same time take place before the rest of what the CIO2 driver does in
> > probe.
> I thought of that as well, but wasn't sure which was preferable. I can
> compress it into the CIO2 driver though sure.

Sakari, I tend to agree with Dan and have the board file separated from the
driver and even framework.

...

> > Cc Andy, too.

Thanks!

...

> >> I wanted to raise this as an RFC as although I don't think it's ready for
> >> integration it has some things that I'd like feedback on, in particular the
> >> method I chose to make the module be auto-inserted. A more ideal method 
> >> would
> >> have been to have the driver be an ACPI driver for the INT343E device, but 
> >> each
> > What do you think this device does represent? Devices whose status is
> > always zero may exist in the table even if they would not be actually
> > present.
> >
> > CIO2 is a PCI device and it has no ACPI (or PNP) ID, or at least should not
> > have one.
> This is the ACPI entry I mean:
> 
> Device (CIO2)
> {
> Method (_STA, 0, NotSerialized)  // _STA: Status
> {
> If ((CIOE == One))
> {
> Return (0x0F)
> }
> Else
> {
> Return (Zero)
> }
> }
> 
> Name (_HID, "INT343E")  // _HID: Hardware ID
> Method (_CRS, 0, NotSerialized)  // _CRS: Current Resource Settings
> {
> Name (CBUF, ResourceTemplate ()
> {
> Interrupt (ResourceConsumer, Level, ActiveLow, Shared, ,, _Y15)
> {
> 0x0010,
> }
> Memory32Fixed (ReadWrite,
> 0xFE40, // Address Base
> 0x0001, // Address Length
> )
> })
> CreateDWordField (CBUF, \_SB.PCI0.CIO2._CRS._Y15._INT, CIOV)  // 
> _INT: Interrupts
> CIOV = CIOI /* \CIOI */
> Return (CBUF) /* \_SB_.PCI0.CIO2._CRS.CBUF */
> }
> }

Ah, I think you misinterpreted the meaning of above. The above is a switch how
camera device appears either as PCI or an ACPI. So, it effectively means you
should *not* have any relation for this HID until you find a platform where the
device is for real enumerated via ACPI.

...

> >> +static int cio2_probe_can_progress(struct pci_dev *pci_dev)
> >> +{
> >> +  void *sensor;

Why void?

> >> +  /*
> >> +   * On ACPI platforms, we need to probe _after_ sensors wishing to 
> >> connect
> >> +   * to cio2 have added a device link. If there are no consumers yet, then
> >> +   * we need to defer. The .sync_state() callback will then be called 
> >> after
> >> +   * all linked sensors have probed
> >> +   */
> >> +
> >> +  if (IS_ENABLED(CONFIG_ACPI)) {

> >> +  sensor = (struct device *)list_first_entry_or_null(

Besides the fact that castings from or to void * are implicit in C, the proper
use of list API should have pretty well defined type of lvalue.

> >> +  
> >> _dev->dev.links.consumers,
> >> +  struct 
> >> dev_links_info,
> >> +  consumers);
> > Please wrap so it's under 80.
> >
> Will do
> >> +
> >> +  if (!sensor)
> >> +  return -EPROBE_DEFER;
> >> +  }
> >> +
> >> +  return 0;
> >> +}

...

> >> +  if (!IS_ENABLED(CONFIG_ACPI)) {
> >> +  r = cio2_parse_firmware(cio2);
> >> +  if (r)
> >> +  goto fail_clean_notifier;
> >> +  }

How comes?

...

> >> \ No newline at end of file

???

Be sure you are using good editor.

...

> >> +#include 

Redundant. ACPI headers are designed the way that you are using a single header
in Linux kernel for all. It might be different in drivers/acpi stuff, but not
in general.

> >> +#include 
> >> +#include 
> >> +#include 
> >> +#include 
> >> +#include 
> >> +#include 
> >> +
> >> +#include 
> >> +#include 

Please, keep them sorted. And since it's for media, the media inclusion may be
placed last in a separate group.

...

> >> +#define PROPERTY_ENTRY_NULL

Re: [RFC PATCH] Add bridge driver to connect sensors to CIO2 device via software nodes on ACPI platforms

2020-09-17 Thread Andy Shevchenko 
On Thu, Sep 17, 2020 at 01:49:41PM +0300, Dan Carpenter wrote:
> On Thu, Sep 17, 2020 at 01:33:43PM +0300, Sakari Ailus wrote:

> > > + int i, ret;
> > 
> > unsigned int i
> > 
> 
> Why?
> 
> For list iterators then "int i;" is best...  For sizes then unsigned is
> sometimes best.  Or if it's part of the hardware spec or network spec
> unsigned is best.  Otherwise unsigned variables cause a ton of bugs.
> They're not as intuitive as signed variables.  Imagine if there is an
> error in this loop and you want to unwind.  With a signed variable you
> can do:
> 
>   while (--i >= 0)
>   cleanup([i]);

Ha-ha. It's actually a counter argument to your stuff because above is the same 
as

while (i--)
cleanup([i]);

with pretty much unsigned int i.

> There are very few times where raising the type maximum from 2 billion
> to 4 billion fixes anything.

-- 
With Best Regards,
Andy Shevchenko


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

Re: [RFC PATCH] Add bridge driver to connect sensors to CIO2 device via software nodes on ACPI platforms

2020-09-17 Thread Dan Scally 
Hey Sakari - thanks for the reply

On 17/09/2020 11:33, Sakari Ailus wrote:
> Hi Daniel,
>
> Thank you for the patch.
>
> Is this all that it takes to add support for some machines shipped with
> Windows?
Almost
> The ones I know require PMIC control done in software (not even
> sensors are accessible without that).
So far we've just been getting the sensor drivers themselves to toggle
the gpio pins that turn the PMIC on (those pins are listed against the
PMIC's _CRS, and we've been finding those by evaluating the sensor's
_DEP) - once that's done the cameras show up on i2c and,with the bridge
driver installed, you can use libcamera to take photos. That's been
confusing me a bit; I think I mentioned before that I couldn't figure
out how the clocks and regulators could be working in that case. But
I've had a bunch of people test this now on about 5 different machines
(Surface devices and similar) and it seems to "just work"
> One possibility would be to put this to platform code. That would
> effectively also require it's compiled to the kernel (yuck).
>
> How about just squashing this to the CIO2 driver instead (but still as a
> separate file)? It's not exactly pretty, no, but it could allow this being
> a module and not enlarge everyone's kernel, and the initialisation would at
> the same time take place before the rest of what the CIO2 driver does in
> probe.
I thought of that as well, but wasn't sure which was preferable. I can
compress it into the CIO2 driver though sure.
> I think you should still check whether CIO2 has graph endpoints before
> proceeding with parsing SSDB buffer or looking up random-looking devices.
Yeah; I was under the impression this wasn't working at all on ACPI
platforms - if it actually is on some then I guess checking for
endpoints before making an attempt to parse SSDB to build them is the
way to go?
>
> Cc Andy, too.
>
> On Wed, Sep 16, 2020 at 10:36:18PM +0100, Daniel Scally wrote:
>> Currently on ACPI platforms, sensors that are intended to be connected to
>> a CIO2 device for use with the ipu3-cio2 driver lack the necessary
>> connection information in firmware. This patch adds a module to parse the
>> connection properties from the SSDB buffer in DSDT and build the connection
>> using software nodes.
>>
>> The ipu3-cio2 driver itself is modified to insert the cio2-bridge module
>> after all sensors that have created a device link between themselves and
>> the CIO2 have probed. Sensors wishing to use this bridge will need to add
>> a device link between themselves and the CIO2 device as part of their own
>> .probe() call.
>>
>> Suggested-by: Jordan Hand 
>>
>> Signed-off-by: Daniel Scally 
>> ---
>> This module's born out of efforts by the linux-surface github community
>> to get functioning webcams on Microsoft Surface and similar platforms. it
>> is dependent on this patch (which implements the software node graph family
>> of functions):
>>
>> https://lore.kernel.org/linux-media/20200915232827.3416-1-djrsca...@gmail.com/
>>
>> I wanted to raise this as an RFC as although I don't think it's ready for
>> integration it has some things that I'd like feedback on, in particular the
>> method I chose to make the module be auto-inserted. A more ideal method would
>> have been to have the driver be an ACPI driver for the INT343E device, but 
>> each
> What do you think this device does represent? Devices whose status is
> always zero may exist in the table even if they would not be actually
> present.
>
> CIO2 is a PCI device and it has no ACPI (or PNP) ID, or at least should not
> have one.
This is the ACPI entry I mean:

Device (CIO2)
{
Method (_STA, 0, NotSerialized)  // _STA: Status
{
If ((CIOE == One))
{
Return (0x0F)
}
Else
{
Return (Zero)
}
}

Name (_HID, "INT343E")  // _HID: Hardware ID
Method (_CRS, 0, NotSerialized)  // _CRS: Current Resource Settings
{
Name (CBUF, ResourceTemplate ()
{
Interrupt (ResourceConsumer, Level, ActiveLow, Shared, ,, _Y15)
{
0x0010,
}
Memory32Fixed (ReadWrite,
0xFE40, // Address Base
0x0001, // Address Length
)
})
CreateDWordField (CBUF, \_SB.PCI0.CIO2._CRS._Y15._INT, CIOV)  // _INT: 
Interrupts
CIOV = CIOI /* \CIOI */
Return (CBUF) /* \_SB_.PCI0.CIO2._CRS.CBUF */
}
}

>> of the the devices we've tested this on that dev has status 0 and so the 
>> module
>> won't bind to it. The device links method seems a little clunky, but does 
>> work,
>> and I think I have done the conditional processing correctly so that 
>> ipu3-cio2
>> continues to work on non-ACPI platforms.
> I don't think anyone uses ipu3-cio2 driver on non-ACPI platforms. It really
> does require ACPI.
Oh - I've been misunderstanding that then. In that case the CONFIG_ACPI
checks can go but I

Re: [RFC PATCH] Add bridge driver to connect sensors to CIO2 device via software nodes on ACPI platforms

2020-09-17 Thread Dan Carpenter 
On Thu, Sep 17, 2020 at 01:33:43PM +0300, Sakari Ailus wrote:
> > +static int connect_supported_devices(void)
> > +{
> > +   struct acpi_device *adev;
> > +   struct device *dev;
> > +   struct sensor_bios_data ssdb;
> > +   struct sensor *sensor;
> > +   struct property_entry *sensor_props;
> > +   struct property_entry *cio2_props;
> > +   struct fwnode_handle *fwnode;
> > +   struct software_node *nodes;
> > +   struct v4l2_subdev *sd;
> > +   int i, ret;
> 
> unsigned int i
> 

Why?

For list iterators then "int i;" is best...  For sizes then unsigned is
sometimes best.  Or if it's part of the hardware spec or network spec
unsigned is best.  Otherwise unsigned variables cause a ton of bugs.
They're not as intuitive as signed variables.  Imagine if there is an
error in this loop and you want to unwind.  With a signed variable you
can do:

while (--i >= 0)
cleanup([i]);

There are very few times where raising the type maximum from 2 billion
to 4 billion fixes anything.

regards,
dan carpenter

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

[PATCH v4 0/8] Add a staging driver for Hikey 970 PHY laywer

2020-09-17 Thread Mauro Carvalho Chehab 
Hi Greg,

This series add the phy layer needed for the USB stack to work
on Hikey 970. 

The main difference against v3 is that I'm sending this one via
staging.

In order for this phy to actually work properly, we still need to
apply one quirk patch at dwc3, which fixes some issues with
USB HID driver. 

I'm working with Felipe and Rob in order to add the quirk on
an approach that would be acceptable for both DT and dwc3
maintainers.

Due to that, I'm not sending the final patch that adds the
the needed dt bindings for this board. 

Thanks!
Mauro

Mauro Carvalho Chehab (6):
  staging: hikey9xx: add build for the Kirin 970 PHY driver
  staging: hikey9xx: phy-hi3670-usb3: use a consistent namespace
  staging: hikey9xx: phy-hi3670-usb3.txt: use a consistent namespace
  staging: hikey9xx: phy-hi3670-usb3: fix coding style
  staging: hikey9xx: phy-hi3670-usb3: change some DT properties
  staging: hikey9xx: convert phy-kirin970-usb3.txt to yaml

Yu Chen (2):
  staging: hikey9xx: add USB physical layer for Kirin 3670
  staging: hikey9xx: phy-hi3670-usb3: fix some issues at the init code

 drivers/staging/hikey9xx/Kconfig  |  11 +
 drivers/staging/hikey9xx/Makefile |   2 +
 drivers/staging/hikey9xx/phy-hi3670-usb3.c| 671 ++
 drivers/staging/hikey9xx/phy-hi3670-usb3.yaml |  72 ++
 4 files changed, 756 insertions(+)
 create mode 100644 drivers/staging/hikey9xx/phy-hi3670-usb3.c
 create mode 100644 drivers/staging/hikey9xx/phy-hi3670-usb3.yaml

-- 
2.26.2


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

[PATCH v4 6/8] staging: hikey9xx: phy-hi3670-usb3: fix coding style

2020-09-17 Thread Mauro Carvalho Chehab 
Address the issues reported by checkpatch --strict,
and add a SPDX tag.

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/staging/hikey9xx/phy-hi3670-usb3.c | 157 ++---
 1 file changed, 76 insertions(+), 81 deletions(-)

diff --git a/drivers/staging/hikey9xx/phy-hi3670-usb3.c 
b/drivers/staging/hikey9xx/phy-hi3670-usb3.c
index fd679b39185a..cb0bfcbbfbfa 100644
--- a/drivers/staging/hikey9xx/phy-hi3670-usb3.c
+++ b/drivers/staging/hikey9xx/phy-hi3670-usb3.c
@@ -1,28 +1,20 @@
+// SPDX-License-Identifier: GPL-2.0-only
 /*
  * Phy provider for USB 3.1 controller on HiSilicon Kirin970 platform
  *
- * Copyright (C) 2017-2018 Hilisicon Electronics Co., Ltd.
+ * Copyright (C) 2017-2020 Hilisicon Electronics Co., Ltd.
  * http://www.huawei.com
  *
  * Authors: Yu Chen 
- *
- * This program is free software: you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2  of
- * the License as published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
  */
 
+#include 
 #include 
 #include 
 #include 
-#include 
 #include 
+#include 
 #include 
-#include 
 
 #define SCTRL_SCDEEPSLEEPED(0x0)
 #define USB_CLK_SELECTED   BIT(20)
@@ -116,7 +108,7 @@
 #define TCPC_VALID BIT(4)
 #define TCPC_LOW_POWER_EN  BIT(3)
 #define TCPC_MUX_CONTROL_MASK  (3 << 0)
-#define TCPC_MUX_CONTROL_USB31 (1 << 0)
+#define TCPC_MUX_CONTROL_USB31 BIT(0)
 
 #define SYSMODE_CFG_TYPEC_DISABLE  BIT(3)
 
@@ -151,13 +143,13 @@ static int hi3670_phy_cr_clk(struct regmap *usb31misc)
 
/* Clock up */
ret = regmap_update_bits(usb31misc, USB_MISC_CFG54,
-   CFG54_USB31PHY_CR_CLK, CFG54_USB31PHY_CR_CLK);
+CFG54_USB31PHY_CR_CLK, CFG54_USB31PHY_CR_CLK);
if (ret)
return ret;
 
/* Clock down */
ret = regmap_update_bits(usb31misc, USB_MISC_CFG54,
-   CFG54_USB31PHY_CR_CLK, 0);
+CFG54_USB31PHY_CR_CLK, 0);
 
return ret;
 }
@@ -165,7 +157,7 @@ static int hi3670_phy_cr_clk(struct regmap *usb31misc)
 static int hi3670_phy_cr_set_sel(struct regmap *usb31misc)
 {
return regmap_update_bits(usb31misc, USB_MISC_CFG54,
-   CFG54_USB31PHY_CR_SEL, CFG54_USB31PHY_CR_SEL);
+ CFG54_USB31PHY_CR_SEL, CFG54_USB31PHY_CR_SEL);
 }
 
 static int hi3670_phy_cr_start(struct regmap *usb31misc, int direction)
@@ -174,10 +166,12 @@ static int hi3670_phy_cr_start(struct regmap *usb31misc, 
int direction)
 
if (direction)
ret = regmap_update_bits(usb31misc, USB_MISC_CFG54,
-   CFG54_USB31PHY_CR_WR_EN, CFG54_USB31PHY_CR_WR_EN);
+CFG54_USB31PHY_CR_WR_EN,
+CFG54_USB31PHY_CR_WR_EN);
else
ret = regmap_update_bits(usb31misc, USB_MISC_CFG54,
-   CFG54_USB31PHY_CR_RD_EN, CFG54_USB31PHY_CR_RD_EN);
+CFG54_USB31PHY_CR_RD_EN,
+CFG54_USB31PHY_CR_RD_EN);
 
if (ret)
return ret;
@@ -187,7 +181,7 @@ static int hi3670_phy_cr_start(struct regmap *usb31misc, 
int direction)
return ret;
 
ret = regmap_update_bits(usb31misc, USB_MISC_CFG54,
-   CFG54_USB31PHY_CR_RD_EN | CFG54_USB31PHY_CR_WR_EN, 0);
+CFG54_USB31PHY_CR_RD_EN | 
CFG54_USB31PHY_CR_WR_EN, 0);
 
return ret;
 }
@@ -223,8 +217,7 @@ static int hi3670_phy_cr_set_addr(struct regmap *usb31misc, 
u32 addr)
return ret;
 
reg &= ~(CFG54_USB31PHY_CR_ADDR_MASK << CFG54_USB31PHY_CR_ADDR_SHIFT);
-   reg |= ((addr & CFG54_USB31PHY_CR_ADDR_MASK) <<
-   CFG54_USB31PHY_CR_ADDR_SHIFT);
+   reg |= ((addr & CFG54_USB31PHY_CR_ADDR_MASK) << 
CFG54_USB31PHY_CR_ADDR_SHIFT);
ret = regmap_write(usb31misc, USB_MISC_CFG54, reg);
 
return ret;
@@ -288,7 +281,7 @@ static int hi3670_phy_cr_write(struct regmap *usb31misc, 
u32 addr, u32 val)
return ret;
 
ret = regmap_write(usb31misc, USB_MISC_CFG58,
-   val & CFG58_USB31PHY_CR_DATA_MASK);
+  val & CFG58_USB31PHY_CR_DATA_MASK);
if (ret)
return ret;
 
@@ -308,7 +301,7 @@ static int hi3670_phy_set_params(struct hi3670_priv *priv)
int retry = 3;
 
ret = regmap_write(priv->usb31misc, USB3OTG_CTRL4,
-   priv->eye_diagram_param);
+  priv->eye_diagram_param);

[PATCH v4 1/8] staging: hikey9xx: add USB physical layer for Kirin 3670

2020-09-17 Thread Mauro Carvalho Chehab 
From: Yu Chen 

Add the Hisilicon Kirin 3670 USB phy driver.

This driver was imported from Linaro's official Hikey 970
tree, from the original patch, removing the addition of
the dwg3-specific parts, and getting the missing SoB from
its original author:


https://github.com/96boards-hikey/linux/commit/9d168f580c9977f9c7f48b228b72035e2f6e3eba#diff-93bb70bc97bdd7be752cb6722adf2124

[mchehab: moved to staging and dropped Makefile/Kconfig changes]
Signed-off-by: Yu Chen 
Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/staging/hikey9xx/phy-hi3670-usb3.c   | 682 +++
 drivers/staging/hikey9xx/phy-hi3670-usb3.txt |  25 +
 2 files changed, 707 insertions(+)
 create mode 100644 drivers/staging/hikey9xx/phy-hi3670-usb3.c
 create mode 100644 drivers/staging/hikey9xx/phy-hi3670-usb3.txt

diff --git a/drivers/staging/hikey9xx/phy-hi3670-usb3.c 
b/drivers/staging/hikey9xx/phy-hi3670-usb3.c
new file mode 100644
index ..4e04ac97728d
--- /dev/null
+++ b/drivers/staging/hikey9xx/phy-hi3670-usb3.c
@@ -0,0 +1,682 @@
+/*
+ * Phy provider for USB 3.1 controller on HiSilicon Kirin970 platform
+ *
+ * Copyright (C) 2017-2018 Hilisicon Electronics Co., Ltd.
+ * http://www.huawei.com
+ *
+ * Authors: Yu Chen 
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2  of
+ * the License as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define SCTRL_SCDEEPSLEEPED(0x0)
+#define USB_CLK_SELECTED   BIT(20)
+
+#define PERI_CRG_PEREN0(0x00)
+#define PERI_CRG_PERDIS0   (0x04)
+#define PERI_CRG_PEREN4(0x40)
+#define PERI_CRG_PERDIS4   (0x44)
+#define PERI_CRG_PERRSTEN4 (0x90)
+#define PERI_CRG_PERRSTDIS4(0x94)
+#define PERI_CRG_ISODIS(0x148)
+#define PERI_CRG_PEREN6(0x410)
+#define PERI_CRG_PERDIS6   (0x414)
+
+#define USB_REFCLK_ISO_EN  BIT(25)
+
+#define GT_CLK_USB2PHY_REF BIT(19)
+
+#define PCTRL_PERI_CTRL3   (0x10)
+#define PCTRL_PERI_CTRL3_MSK_START (16)
+#define USB_TCXO_ENBIT(1)
+
+#define PCTRL_PERI_CTRL24  (0x64)
+#define SC_CLK_USB3PHY_3MUX1_SEL   BIT(25)
+
+#define USB3OTG_CTRL0  (0x00)
+#define USB3OTG_CTRL3  (0x0C)
+#define USB3OTG_CTRL4  (0x10)
+#define USB3OTG_CTRL5  (0x14)
+#define USB3OTG_CTRL7  (0x1C)
+#define USB_MISC_CFG50 (0x50)
+#define USB_MISC_CFG54 (0x54)
+#define USB_MISC_CFG58 (0x58)
+#define USB_MISC_CFG5C (0x5C)
+#define USB_MISC_CFGA0 (0xA0)
+#define TCA_CLK_RST(0x200)
+#define TCA_INTR_EN(0x204)
+#define TCA_INTR_STS   (0x208)
+#define TCA_GCFG   (0x210)
+#define TCA_TCPC   (0x214)
+#define TCA_VBUS_CTRL  (0x240)
+
+#define CTRL0_USB3_VBUSVLD BIT(7)
+#define CTRL0_USB3_VBUSVLD_SEL BIT(6)
+
+#define CTRL3_USB2_VBUSVLDEXT0 BIT(6)
+#define CTRL3_USB2_VBUSVLDEXTSEL0  BIT(5)
+
+#define CTRL5_USB2_SIDDQ   BIT(0)
+
+#define CTRL7_USB2_REFCLKSEL_MASK  (3 << 3)
+#define CTRL7_USB2_REFCLKSEL_ABB   (3 << 3)
+#define CTRL7_USB2_REFCLKSEL_PAD   (2 << 3)
+
+#define CFG50_USB3_PHY_TEST_POWERDOWN  BIT(23)
+
+#define CFG54_USB31PHY_CR_ADDR_MASK(0x)
+#define CFG54_USB31PHY_CR_ADDR_SHIFT   (16)
+#define CFG54_USB3PHY_REF_USE_PAD  BIT(12)
+#define CFG54_PHY0_PMA_PWR_STABLE  BIT(11)
+#define CFG54_PHY0_PCS_PWR_STABLE  BIT(9)
+#define CFG54_USB31PHY_CR_ACK  BIT(7)
+#define CFG54_USB31PHY_CR_WR_ENBIT(5)
+#define CFG54_USB31PHY_CR_SEL  BIT(4)
+#define CFG54_USB31PHY_CR_RD_ENBIT(3)
+#define CFG54_USB31PHY_CR_CLK  BIT(2)
+#define CFG54_USB3_PHY0_ANA_PWR_EN BIT(1)
+
+#define CFG58_USB31PHY_CR_DATA_MASK (0x)
+#define CFG58_USB31PHY_CR_DATA_RD_START (16)
+
+#define CFG5C_USB3_PHY0_SS_MPLLA_SSC_ENBIT(1)
+
+#define CFGA0_VAUX_RESET   BIT(9)
+#define CFGA0_USB31C_RESET BIT(8)
+#define CFGA0_USB2PHY_REFCLK_SELECTBIT(4)
+#define CFGA0_USB3PHY_RESETBIT(1)
+#define CFGA0_USB2PHY_POR  BIT(0)
+
+#define INTR_EN_XA_TIMEOUT_EVT_EN  BIT(1)
+#define INTR_EN_XA_ACK_EVT_EN  BIT(0)
+
+#define CLK_RST_TCA_REF_CLK_EN BIT(1)
+#define

[PATCH v4 2/8] staging: hikey9xx: add build for the Kirin 970 PHY driver

2020-09-17 Thread Mauro Carvalho Chehab 
Add the needed bits in order to build the Kirin 970 PHY
driver.

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/staging/hikey9xx/Kconfig  | 11 +++
 drivers/staging/hikey9xx/Makefile |  2 ++
 2 files changed, 13 insertions(+)

diff --git a/drivers/staging/hikey9xx/Kconfig b/drivers/staging/hikey9xx/Kconfig
index b2ce886e1c4e..e3d1c00394cd 100644
--- a/drivers/staging/hikey9xx/Kconfig
+++ b/drivers/staging/hikey9xx/Kconfig
@@ -1,5 +1,16 @@
 # SPDX-License-Identifier: GPL-2.0
 
+# to be placed at drivers/phy
+config PHY_HI3670_USB
+   tristate "hi3670 USB PHY support"
+   depends on (ARCH_HISI && ARM64) || COMPILE_TEST
+   select GENERIC_PHY
+   select MFD_SYSCON
+   help
+ Enable this to support the HISILICON HI3670 USB PHY.
+
+ To compile this driver as a module, choose M here.
+
 # to be placed at drivers/spmi
 config SPMI_HISI3670
tristate "Hisilicon 3670 SPMI Controller"
diff --git a/drivers/staging/hikey9xx/Makefile 
b/drivers/staging/hikey9xx/Makefile
index 1a848d398ab6..7f3e86dddb38 100644
--- a/drivers/staging/hikey9xx/Makefile
+++ b/drivers/staging/hikey9xx/Makefile
@@ -1,5 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0
 
+obj-$(CONFIG_PHY_HI3670_USB)   += phy-hi3670-usb3.o
+
 obj-$(CONFIG_SPMI_HISI3670)+= hisi-spmi-controller.o
 obj-$(CONFIG_MFD_HI6421_SPMI)  += hi6421-spmi-pmic.o
 obj-$(CONFIG_REGULATOR_HI6421V600) += hi6421v600-regulator.o
-- 
2.26.2

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

[PATCH v4 5/8] staging: hikey9xx: phy-hi3670-usb3.txt: use a consistent namespace

2020-09-17 Thread Mauro Carvalho Chehab 
While this driver is not used yet, use a more consistent namespace,
similar to the PHY layer for Kirin 960 (hi3660).

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/staging/hikey9xx/phy-hi3670-usb3.txt | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/hikey9xx/phy-hi3670-usb3.txt 
b/drivers/staging/hikey9xx/phy-hi3670-usb3.txt
index 4cb02612ff23..2fb27cb8beaf 100644
--- a/drivers/staging/hikey9xx/phy-hi3670-usb3.txt
+++ b/drivers/staging/hikey9xx/phy-hi3670-usb3.txt
@@ -2,7 +2,7 @@ Hisilicon Kirin970 usb PHY
 ---
 
 Required properties:
-- compatible: should be "hisilicon,kirin970-usb-phy"
+- compatible: should be "hisilicon,hi3670-usb-phy"
 - #phy-cells: must be 0
 - hisilicon,pericrg-syscon: phandle of syscon used to control phy.
 - hisilicon,pctrl-syscon: phandle of syscon used to control phy.
@@ -14,7 +14,7 @@ Refer to phy/phy-bindings.txt for the generic PHY binding 
properties
 
 Example:
usb_phy: usbphy {
-   compatible = "hisilicon,kirin970-usb-phy";
+   compatible = "hisilicon,hi3670-usb-phy";
#phy-cells = <0>;
hisilicon,pericrg-syscon = <_ctrl>;
hisilicon,pctrl-syscon = <>;
-- 
2.26.2

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

[PATCH v4 3/8] staging: hikey9xx: phy-hi3670-usb3: fix some issues at the init code

2020-09-17 Thread Mauro Carvalho Chehab 
From: Yu Chen 

There are some problems at the initialization part of this phy.
Solve them.

Signed-off-by: Yu Chen 
Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/staging/hikey9xx/phy-hi3670-usb3.c | 70 ++
 1 file changed, 32 insertions(+), 38 deletions(-)

diff --git a/drivers/staging/hikey9xx/phy-hi3670-usb3.c 
b/drivers/staging/hikey9xx/phy-hi3670-usb3.c
index 4e04ac97728d..1d4caf7a2aaf 100644
--- a/drivers/staging/hikey9xx/phy-hi3670-usb3.c
+++ b/drivers/staging/hikey9xx/phy-hi3670-usb3.c
@@ -63,6 +63,7 @@
 #define TCA_INTR_STS   (0x208)
 #define TCA_GCFG   (0x210)
 #define TCA_TCPC   (0x214)
+#define TCA_SYSMODE_CFG(0x218)
 #define TCA_VBUS_CTRL  (0x240)
 
 #define CTRL0_USB3_VBUSVLD BIT(7)
@@ -109,12 +110,16 @@
 #define CLK_RST_SUSPEND_CLK_EN BIT(0)
 
 #define GCFG_ROLE_HSTDEV   BIT(4)
+#define GCFG_OP_MODE   (3 << 0)
+#define GCFG_OP_MODE_CTRL_SYNC_MODEBIT(0)
 
 #define TCPC_VALID BIT(4)
 #define TCPC_LOW_POWER_EN  BIT(3)
 #define TCPC_MUX_CONTROL_MASK  (3 << 0)
 #define TCPC_MUX_CONTROL_USB31 (1 << 0)
 
+#define SYSMODE_CFG_TYPEC_DISABLE  BIT(3)
+
 #define VBUS_CTRL_POWERPRESENT_OVERRD  (3 << 2)
 #define VBUS_CTRL_VBUSVALID_OVERRD (3 << 0)
 
@@ -363,6 +368,11 @@ static int kirin970_config_phy_clock(struct kirin970_priv 
*priv)
if (ret)
goto out;
 
+   /* enable usb_tcxo_en */
+   ret = regmap_write(priv->pctrl, PCTRL_PERI_CTRL3,
+   USB_TCXO_EN |
+   (USB_TCXO_EN << PCTRL_PERI_CTRL3_MSK_START));
+
/* select usbphy clk from abb */
mask = SC_CLK_USB3PHY_3MUX1_SEL;
ret = regmap_update_bits(priv->pctrl,
@@ -437,7 +447,13 @@ static int kirin970_config_tca(struct kirin970_priv *priv)
goto out;
 
ret = regmap_update_bits(priv->usb31misc, TCA_GCFG,
-   GCFG_ROLE_HSTDEV, GCFG_ROLE_HSTDEV);
+   GCFG_ROLE_HSTDEV | GCFG_OP_MODE,
+   GCFG_ROLE_HSTDEV | GCFG_OP_MODE_CTRL_SYNC_MODE);
+   if (ret)
+   goto out;
+
+   ret = regmap_update_bits(priv->usb31misc, TCA_SYSMODE_CFG,
+   SYSMODE_CFG_TYPEC_DISABLE, 0);
if (ret)
goto out;
 
@@ -461,18 +477,15 @@ static int kirin970_config_tca(struct kirin970_priv *priv)
return ret;
 }
 
-static int kirin970_phy_exit(struct phy *phy);
-
 static int kirin970_phy_init(struct phy *phy)
 {
struct kirin970_priv *priv = phy_get_drvdata(phy);
u32 val;
int ret;
 
-   kirin970_phy_exit(phy);
-   dev_info(priv->dev, "%s in\n", __func__);
/* assert controller */
-   val = CFGA0_VAUX_RESET | CFGA0_USB31C_RESET;
+   val = CFGA0_VAUX_RESET | CFGA0_USB31C_RESET |
+   CFGA0_USB3PHY_RESET | CFGA0_USB2PHY_POR;
ret = regmap_update_bits(priv->usb31misc, USB_MISC_CFGA0, val, 0);
if (ret)
goto out;
@@ -493,6 +506,14 @@ static int kirin970_phy_init(struct phy *phy)
if (ret)
goto out;
 
+   /* Deassert phy */
+   val = CFGA0_USB3PHY_RESET | CFGA0_USB2PHY_POR;
+   ret = regmap_update_bits(priv->usb31misc, USB_MISC_CFGA0, val, val);
+   if (ret)
+   goto out;
+
+   udelay(100);
+
/* Tell the PHY power is stable */
val = CFG54_USB3_PHY0_ANA_PWR_EN | CFG54_PHY0_PCS_PWR_STABLE |
CFG54_PHY0_PMA_PWR_STABLE;
@@ -512,14 +533,6 @@ static int kirin970_phy_init(struct phy *phy)
if (ret)
goto out;
 
-   /* Deassert phy */
-   val = CFGA0_USB3PHY_RESET | CFGA0_USB2PHY_POR;
-   ret = regmap_update_bits(priv->usb31misc, USB_MISC_CFGA0, val, val);
-   if (ret)
-   goto out;
-
-   udelay(100);
-
/* Deassert controller */
val = CFGA0_VAUX_RESET | CFGA0_USB31C_RESET;
ret = regmap_update_bits(priv->usb31misc, USB_MISC_CFGA0, val, val);
@@ -545,29 +558,6 @@ static int kirin970_phy_init(struct phy *phy)
if (ret)
goto out;
 
-   {
-   ret = regmap_read(priv->peri_crg, 0x4c,
-   );
-   if (!ret)
-   dev_info(priv->dev, "peri_crg 0x4c %x\n", val);
-   ret = regmap_read(priv->peri_crg, 0x404,
-   );
-   if (!ret)
-   dev_info(priv->dev, "peri_crg 0x404 %x\n", val);
-   ret = regmap_read(priv->peri_crg, 0xc,
-   );
-   if (!ret)
-   dev_info(priv->dev, "peri_crg 0xc %x\n", val);
-   ret = regmap_read(priv->peri_crg, 0xac,
-   );
-   if

[PATCH v4 8/8] staging: hikey9xx: convert phy-kirin970-usb3.txt to yaml

2020-09-17 Thread Mauro Carvalho Chehab 
Use the new YAML for this physical layer.

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/staging/hikey9xx/phy-hi3670-usb3.txt  | 25 ---
 drivers/staging/hikey9xx/phy-hi3670-usb3.yaml | 72 +++
 2 files changed, 72 insertions(+), 25 deletions(-)
 delete mode 100644 drivers/staging/hikey9xx/phy-hi3670-usb3.txt
 create mode 100644 drivers/staging/hikey9xx/phy-hi3670-usb3.yaml

diff --git a/drivers/staging/hikey9xx/phy-hi3670-usb3.txt 
b/drivers/staging/hikey9xx/phy-hi3670-usb3.txt
deleted file mode 100644
index 2fb27cb8beaf..
--- a/drivers/staging/hikey9xx/phy-hi3670-usb3.txt
+++ /dev/null
@@ -1,25 +0,0 @@
-Hisilicon Kirin970 usb PHY

-
-Required properties:
-- compatible: should be "hisilicon,hi3670-usb-phy"
-- #phy-cells: must be 0
-- hisilicon,pericrg-syscon: phandle of syscon used to control phy.
-- hisilicon,pctrl-syscon: phandle of syscon used to control phy.
-- hisilicon,sctrl-syscon: phandle of syscon used to control phy.
-- hisilicon,usb31-misc-syscon: phandle of syscon used to control phy.
-- eye-diagram-param: parameter set for phy
-- usb3-phy-tx-vboost-lvl: parameter set for phy
-Refer to phy/phy-bindings.txt for the generic PHY binding properties
-
-Example:
-   usb_phy: usbphy {
-   compatible = "hisilicon,hi3670-usb-phy";
-   #phy-cells = <0>;
-   hisilicon,pericrg-syscon = <_ctrl>;
-   hisilicon,pctrl-syscon = <>;
-   hisilicon,sctrl-syscon = <>;
-   hisilicon,usb31-misc-syscon = <_misc>;
-   eye-diagram-param = <0xFDFEE4>;
-   usb3-phy-tx-vboost-lvl = <0x5>;
-   };
diff --git a/drivers/staging/hikey9xx/phy-hi3670-usb3.yaml 
b/drivers/staging/hikey9xx/phy-hi3670-usb3.yaml
new file mode 100644
index ..125a5d6546ae
--- /dev/null
+++ b/drivers/staging/hikey9xx/phy-hi3670-usb3.yaml
@@ -0,0 +1,72 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/phy/hisilicon,hi3670-usb3.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Hisilicon Kirin970 USB PHY
+
+maintainers:
+  - Mauro Carvalho Chehab 
+description: |+
+  Bindings for USB3 PHY on HiSilicon Kirin 970.
+
+properties:
+  compatible:
+const: hisilicon,hi3670-usb-phy
+
+  "#phy-cells":
+const: 0
+
+  hisilicon,pericrg-syscon:
+$ref: '/schemas/types.yaml#/definitions/phandle'
+description: phandle of syscon used to control iso refclk.
+
+  hisilicon,pctrl-syscon:
+$ref: '/schemas/types.yaml#/definitions/phandle'
+description: phandle of syscon used to control usb tcxo.
+
+  hisilicon,sctrl-syscon:
+$ref: '/schemas/types.yaml#/definitions/phandle'
+description: phandle of syscon used to control phy deep sleep.
+
+  hisilicon,eye-diagram-param:
+$ref: /schemas/types.yaml#/definitions/uint32
+description: Eye diagram for phy.
+
+  hisilicon,tx-vboost-lvl:
+$ref: /schemas/types.yaml#/definitions/uint32
+description: TX level vboost for phy.
+
+required:
+  - compatible
+  - hisilicon,pericrg-syscon
+  - hisilicon,pctrl-syscon
+  - hisilicon,sctrl-syscon
+  - hisilicon,eye-diagram-param
+  - hisilicon,tx-vboost-lvl
+  - "#phy-cells"
+
+additionalProperties: false
+
+examples:
+  - |
+bus {
+  #address-cells = <2>;
+  #size-cells = <2>;
+
+  usb3_otg_bc: usb3_otg_bc@ff20 {
+compatible = "syscon", "simple-mfd";
+reg = <0x0 0xff20 0x0 0x1000>;
+
+usb_phy {
+  compatible = "hisilicon,hi3670-usb-phy";
+  #phy-cells = <0>;
+  hisilicon,pericrg-syscon = <_ctrl>;
+  hisilicon,pctrl-syscon = <>;
+  hisilicon,sctrl-syscon = <>;
+  hisilicon,eye-diagram-param = <0xfdfee4>;
+  hisilicon,tx-vboost-lvl = <0x5>;
+};
+  };
+};
-- 
2.26.2

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

[PATCH v4 4/8] staging: hikey9xx: phy-hi3670-usb3: use a consistent namespace

2020-09-17 Thread Mauro Carvalho Chehab 
Rename hikey970 to hi3670, in order to use a namespace
similar to hi3660 driver.

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/staging/hikey9xx/phy-hi3670-usb3.c | 98 +++---
 1 file changed, 49 insertions(+), 49 deletions(-)

diff --git a/drivers/staging/hikey9xx/phy-hi3670-usb3.c 
b/drivers/staging/hikey9xx/phy-hi3670-usb3.c
index 1d4caf7a2aaf..fd679b39185a 100644
--- a/drivers/staging/hikey9xx/phy-hi3670-usb3.c
+++ b/drivers/staging/hikey9xx/phy-hi3670-usb3.c
@@ -130,7 +130,7 @@
 #define TX_VBOOST_LVL_START(6)
 #define TX_VBOOST_LVL_ENABLE   BIT(9)
 
-struct kirin970_priv {
+struct hi3670_priv {
struct device *dev;
struct regmap *peri_crg;
struct regmap *pctrl;
@@ -145,7 +145,7 @@ struct kirin970_priv {
u32 usb31misc_offset;
 };
 
-static int kirin970_phy_cr_clk(struct regmap *usb31misc)
+static int hi3670_phy_cr_clk(struct regmap *usb31misc)
 {
int ret;
 
@@ -162,13 +162,13 @@ static int kirin970_phy_cr_clk(struct regmap *usb31misc)
return ret;
 }
 
-static int kirin970_phy_cr_set_sel(struct regmap *usb31misc)
+static int hi3670_phy_cr_set_sel(struct regmap *usb31misc)
 {
return regmap_update_bits(usb31misc, USB_MISC_CFG54,
CFG54_USB31PHY_CR_SEL, CFG54_USB31PHY_CR_SEL);
 }
 
-static int kirin970_phy_cr_start(struct regmap *usb31misc, int direction)
+static int hi3670_phy_cr_start(struct regmap *usb31misc, int direction)
 {
int ret;
 
@@ -182,7 +182,7 @@ static int kirin970_phy_cr_start(struct regmap *usb31misc, 
int direction)
if (ret)
return ret;
 
-   ret = kirin970_phy_cr_clk(usb31misc);
+   ret = hi3670_phy_cr_clk(usb31misc);
if (ret)
return ret;
 
@@ -192,7 +192,7 @@ static int kirin970_phy_cr_start(struct regmap *usb31misc, 
int direction)
return ret;
 }
 
-static int kirin970_phy_cr_wait_ack(struct regmap *usb31misc)
+static int hi3670_phy_cr_wait_ack(struct regmap *usb31misc)
 {
u32 reg;
int retry = 10;
@@ -205,7 +205,7 @@ static int kirin970_phy_cr_wait_ack(struct regmap 
*usb31misc)
if ((reg & CFG54_USB31PHY_CR_ACK) == CFG54_USB31PHY_CR_ACK)
return 0;
 
-   ret = kirin970_phy_cr_clk(usb31misc);
+   ret = hi3670_phy_cr_clk(usb31misc);
if (ret)
return ret;
}
@@ -213,7 +213,7 @@ static int kirin970_phy_cr_wait_ack(struct regmap 
*usb31misc)
return -ETIMEDOUT;
 }
 
-static int kirin970_phy_cr_set_addr(struct regmap *usb31misc, u32 addr)
+static int hi3670_phy_cr_set_addr(struct regmap *usb31misc, u32 addr)
 {
u32 reg;
int ret;
@@ -230,31 +230,31 @@ static int kirin970_phy_cr_set_addr(struct regmap 
*usb31misc, u32 addr)
return ret;
 }
 
-static int kirin970_phy_cr_read(struct regmap *usb31misc, u32 addr, u32 *val)
+static int hi3670_phy_cr_read(struct regmap *usb31misc, u32 addr, u32 *val)
 {
int reg;
int i;
int ret;
 
for (i = 0; i < 100; i++) {
-   ret = kirin970_phy_cr_clk(usb31misc);
+   ret = hi3670_phy_cr_clk(usb31misc);
if (ret)
return ret;
}
 
-   ret = kirin970_phy_cr_set_sel(usb31misc);
+   ret = hi3670_phy_cr_set_sel(usb31misc);
if (ret)
return ret;
 
-   ret = kirin970_phy_cr_set_addr(usb31misc, addr);
+   ret = hi3670_phy_cr_set_addr(usb31misc, addr);
if (ret)
return ret;
 
-   ret = kirin970_phy_cr_start(usb31misc, 0);
+   ret = hi3670_phy_cr_start(usb31misc, 0);
if (ret)
return ret;
 
-   ret = kirin970_phy_cr_wait_ack(usb31misc);
+   ret = hi3670_phy_cr_wait_ack(usb31misc);
if (ret)
return ret;
 
@@ -268,22 +268,22 @@ static int kirin970_phy_cr_read(struct regmap *usb31misc, 
u32 addr, u32 *val)
return 0;
 }
 
-static int kirin970_phy_cr_write(struct regmap *usb31misc, u32 addr, u32 val)
+static int hi3670_phy_cr_write(struct regmap *usb31misc, u32 addr, u32 val)
 {
int i;
int ret;
 
for (i = 0; i < 100; i++) {
-   ret = kirin970_phy_cr_clk(usb31misc);
+   ret = hi3670_phy_cr_clk(usb31misc);
if (ret)
return ret;
}
 
-   ret = kirin970_phy_cr_set_sel(usb31misc);
+   ret = hi3670_phy_cr_set_sel(usb31misc);
if (ret)
return ret;
 
-   ret = kirin970_phy_cr_set_addr(usb31misc, addr);
+   ret = hi3670_phy_cr_set_addr(usb31misc, addr);
if (ret)
return ret;
 
@@ -292,16 +292,16 @@ static int kirin970_phy_cr_write(struct regmap 
*usb31misc, u32 addr, u32 val)
if (ret)
return ret;
 
-   ret = kirin970_phy_cr_start(usb31misc, 1);
+   ret = hi3670_phy_cr_start(usb31misc, 1);
if (ret)
return

[PATCH v4 7/8] staging: hikey9xx: phy-hi3670-usb3: change some DT properties

2020-09-17 Thread Mauro Carvalho Chehab 
Do some changes at the DT properties in order to make it
follow the phy-hi3660-usb3 example and to simplify
usb3-phy-tx-vboost-lvl name.

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/staging/hikey9xx/phy-hi3670-usb3.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/hikey9xx/phy-hi3670-usb3.c 
b/drivers/staging/hikey9xx/phy-hi3670-usb3.c
index cb0bfcbbfbfa..42dbc20a0b9a 100644
--- a/drivers/staging/hikey9xx/phy-hi3670-usb3.c
+++ b/drivers/staging/hikey9xx/phy-hi3670-usb3.c
@@ -627,18 +627,18 @@ static int hi3670_phy_probe(struct platform_device *pdev)
return PTR_ERR(priv->sctrl);
}
 
-   priv->usb31misc = syscon_regmap_lookup_by_phandle(dev->of_node,
- 
"hisilicon,usb31-misc-syscon");
+   /* node of hi3670 phy is a sub-node of usb3_otg_bc */
+   priv->usb31misc = syscon_node_to_regmap(dev->parent->of_node);
if (IS_ERR(priv->usb31misc)) {
-   dev_err(dev, "no hisilicon,usb31-misc-syscon\n");
+   dev_err(dev, "no hisilicon,usb3-otg-bc-syscon\n");
return PTR_ERR(priv->usb31misc);
}
 
-   if (of_property_read_u32(dev->of_node, "eye-diagram-param",
+   if (of_property_read_u32(dev->of_node, "hisilicon,eye-diagram-param",
 >eye_diagram_param))
priv->eye_diagram_param = KIRIN970_USB_DEFAULT_PHY_PARAM;
 
-   if (of_property_read_u32(dev->of_node, "usb3-phy-tx-vboost-lvl",
+   if (of_property_read_u32(dev->of_node, "hisilicon,tx-vboost-lvl",
 >tx_vboost_lvl))
priv->eye_diagram_param = KIRIN970_USB_DEFAULT_PHY_VBOOST;
 
-- 
2.26.2

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

Re: [PATCH] media: cedrus: Propagate OUTPUT resolution to CAPTURE

2020-09-17 Thread Hans Verkuil 
On 14/05/2020 17:39, Nicolas Dufresne wrote:
> As per spec, the CAPTURE resolution should be automatically set based on
> the OTUPUT resolution. This patch properly propagate width/height to the
> capture when the OUTPUT format is set and override the user provided
> width/height with configured OUTPUT resolution when the CAPTURE fmt is
> updated.
> 
> This also prevents userspace from selecting a CAPTURE resolution that is
> too small, avoiding unwanted page faults.
> 
> Signed-off-by: Nicolas Dufresne 
> ---
>  drivers/staging/media/sunxi/cedrus/cedrus_video.c | 7 ++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_video.c 
> b/drivers/staging/media/sunxi/cedrus/cedrus_video.c
> index 16d82309e7b6..a6d6b15adc2e 100644
> --- a/drivers/staging/media/sunxi/cedrus/cedrus_video.c
> +++ b/drivers/staging/media/sunxi/cedrus/cedrus_video.c
> @@ -247,6 +247,8 @@ static int cedrus_try_fmt_vid_cap(struct file *file, void 
> *priv,
>   return -EINVAL;
>  
>   pix_fmt->pixelformat = fmt->pixelformat;
> + pix_fmt->width = ctx->src_fmt.width;
> + pix_fmt->height = ctx->src_fmt.height;
>   cedrus_prepare_format(pix_fmt);
>  
>   return 0;
> @@ -319,11 +321,14 @@ static int cedrus_s_fmt_vid_out(struct file *file, void 
> *priv,
>   break;
>   }
>  
> - /* Propagate colorspace information to capture. */
> + /* Propagate format information to capture. */
>   ctx->dst_fmt.colorspace = f->fmt.pix.colorspace;
>   ctx->dst_fmt.xfer_func = f->fmt.pix.xfer_func;
>   ctx->dst_fmt.ycbcr_enc = f->fmt.pix.ycbcr_enc;
>   ctx->dst_fmt.quantization = f->fmt.pix.quantization;
> + ctx->dst_fmt.width = ctx->src_fmt.width;
> + ctx->dst_fmt.height = ctx->src_fmt.height;

You can only do this if the capture queue isn't busy.

See also hantro_reset_raw_fmt() where it does that check.

So this needs a v2.

Regards,

Hans

> + cedrus_prepare_format(>dst_fmt);
>  
>   return 0;
>  }
> 

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

Re: [RFC PATCH] Add bridge driver to connect sensors to CIO2 device via software nodes on ACPI platforms

2020-09-17 Thread Joe Perches 
On Thu, 2020-09-17 at 12:34 +0300, Dan Carpenter wrote:
> On Wed, Sep 16, 2020 at 10:36:18PM +0100, Daniel Scally wrote:
> > diff --git a/drivers/staging/media/ipu3/cio2-bridge.c 
> > b/drivers/staging/media/ipu3/cio2-bridge.c
[]
> > +   if (!dev->driver_data) {
> > +   pr_info("ACPI match for %s, but it has no driver\n",
> > +   supported_devices[i]);
> 
> put_device(dev);
> 
> > +   continue;
> > +   } else {

No need for an else either.

> > +   pr_info("Found supported device %s\n",
> > +   supported_devices[i]);

so this can be unindented too.


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

Re: [RFC PATCH] Add bridge driver to connect sensors to CIO2 device via software nodes on ACPI platforms

2020-09-17 Thread Sakari Ailus 
Hi Daniel,

Thank you for the patch.

Is this all that it takes to add support for some machines shipped with
Windows? The ones I know require PMIC control done in software (not even
sensors are accessible without that).

One possibility would be to put this to platform code. That would
effectively also require it's compiled to the kernel (yuck).

How about just squashing this to the CIO2 driver instead (but still as a
separate file)? It's not exactly pretty, no, but it could allow this being
a module and not enlarge everyone's kernel, and the initialisation would at
the same time take place before the rest of what the CIO2 driver does in
probe.

I think you should still check whether CIO2 has graph endpoints before
proceeding with parsing SSDB buffer or looking up random-looking devices.

Cc Andy, too.

On Wed, Sep 16, 2020 at 10:36:18PM +0100, Daniel Scally wrote:
> Currently on ACPI platforms, sensors that are intended to be connected to
> a CIO2 device for use with the ipu3-cio2 driver lack the necessary
> connection information in firmware. This patch adds a module to parse the
> connection properties from the SSDB buffer in DSDT and build the connection
> using software nodes.
> 
> The ipu3-cio2 driver itself is modified to insert the cio2-bridge module
> after all sensors that have created a device link between themselves and
> the CIO2 have probed. Sensors wishing to use this bridge will need to add
> a device link between themselves and the CIO2 device as part of their own
> .probe() call.
> 
> Suggested-by: Jordan Hand 
> 
> Signed-off-by: Daniel Scally 
> ---
> This module's born out of efforts by the linux-surface github community
> to get functioning webcams on Microsoft Surface and similar platforms. it
> is dependent on this patch (which implements the software node graph family
> of functions):
> 
> https://lore.kernel.org/linux-media/20200915232827.3416-1-djrsca...@gmail.com/
> 
> I wanted to raise this as an RFC as although I don't think it's ready for
> integration it has some things that I'd like feedback on, in particular the
> method I chose to make the module be auto-inserted. A more ideal method would
> have been to have the driver be an ACPI driver for the INT343E device, but 
> each

What do you think this device does represent? Devices whose status is
always zero may exist in the table even if they would not be actually
present.

CIO2 is a PCI device and it has no ACPI (or PNP) ID, or at least should not
have one.

> of the the devices we've tested this on that dev has status 0 and so the 
> module
> won't bind to it. The device links method seems a little clunky, but does 
> work,
> and I think I have done the conditional processing correctly so that ipu3-cio2
> continues to work on non-ACPI platforms.

I don't think anyone uses ipu3-cio2 driver on non-ACPI platforms. It really
does require ACPI.

> 
>  MAINTAINERS  |   6 +
>  drivers/media/pci/intel/ipu3/ipu3-cio2.c |  67 +++-
>  drivers/staging/media/ipu3/Kconfig   |  15 +
>  drivers/staging/media/ipu3/Makefile  |   1 +
>  drivers/staging/media/ipu3/cio2-bridge.c | 448 +++
>  5 files changed, 534 insertions(+), 3 deletions(-)
>  create mode 100644 drivers/staging/media/ipu3/cio2-bridge.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index b5cfab015bd6..55b0b9888bc0 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -9152,6 +9152,12 @@ S: Maintained
>  W:   http://www.adaptec.com/
>  F:   drivers/scsi/ips*
>  
> +IPU3 CIO2 Bridge Driver
> +M:   Daniel Scally 
> +L:   linux-me...@vger.kernel.org
> +S:   Maintained
> +F:   drivers/staging/media/ipu3/cio2-bridge.c
> +
>  IPVS
>  M:   Wensong Zhang 
>  M:   Simon Horman 
> diff --git a/drivers/media/pci/intel/ipu3/ipu3-cio2.c 
> b/drivers/media/pci/intel/ipu3/ipu3-cio2.c
> index 92f5eadf2c99..fd941d2c7581 100644
> --- a/drivers/media/pci/intel/ipu3/ipu3-cio2.c
> +++ b/drivers/media/pci/intel/ipu3/ipu3-cio2.c
> @@ -1719,6 +1719,59 @@ static void cio2_queues_exit(struct cio2_device *cio2)
>   cio2_queue_exit(cio2, >queue[i]);
>  }
>  
> +static int cio2_probe_can_progress(struct pci_dev *pci_dev)
> +{
> + void *sensor;
> +
> + /*
> +  * On ACPI platforms, we need to probe _after_ sensors wishing to 
> connect
> +  * to cio2 have added a device link. If there are no consumers yet, then
> +  * we need to defer. The .sync_state() callback will then be called 
> after
> +  * all linked sensors have probed
> +  */
> +
> + if (IS_ENABLED(CONFIG_ACPI)) {
> + sensor = (struct device *)list_first_entry_or_null(
> + 
> _dev->dev.links.consumers,
> + struct 
> dev_links_info,
> + consumers);

Please wrap so it's under 80.

> +
> + if (!sensor)
> + return -EPROBE_DEFER;
>

Re: [RFC PATCH] Add bridge driver to connect sensors to CIO2 device via software nodes on ACPI platforms

2020-09-17 Thread Dan Scally 
On 17/09/2020 11:15, Dan Carpenter wrote:
> On Thu, Sep 17, 2020 at 10:47:50AM +0100, Dan Scally wrote:
>> Hi Greg - thanks for the comments, appreciate it (sorry there's so many,
>> I'm new to both C and kernel work)
> It's pretty impressive work if you're new to C...
Thanks (and for your other reply too, haven't had time to go through it
in depth yet). I've been using python for a while, but this is my first
attempt at anything serious with C.
 +  return;
>>> No error value?
>> The prototype for sync_state callbacks is to return void, so my
>> understanding is it can't return an error value.  I guess a better thing
>> to do might be call another function performing cleanup and unloading
>> the driver before the return or something along those lines though.
> Yeah.  I suspect you should be using a different callback instead of
> ->sync_state() but I don't know what... :/
Yeah, this is why I wanted to submit it now really; I too suspect I
should really be using a different callback but I couldn't find a better
option.
> regards,
> dan carpenter
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [RFC PATCH] Add bridge driver to connect sensors to CIO2 device via software nodes on ACPI platforms

2020-09-17 Thread Dan Carpenter 
On Thu, Sep 17, 2020 at 10:47:50AM +0100, Dan Scally wrote:
> Hi Greg - thanks for the comments, appreciate it (sorry there's so many,
> I'm new to both C and kernel work)

It's pretty impressive work if you're new to C...

> >
> >> +  return;
> > No error value?
> The prototype for sync_state callbacks is to return void, so my
> understanding is it can't return an error value.  I guess a better thing
> to do might be call another function performing cleanup and unloading
> the driver before the return or something along those lines though.

Yeah.  I suspect you should be using a different callback instead of
->sync_state() but I don't know what... :/

regards,
dan carpenter

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

[PATCH v16 0/2] Add initial support for slimport anx7625

2020-09-17 Thread Xin Ji 
Hi all,

The following series add support for the Slimport ANX7625 transmitter, a
ultra-low power Full-HD 4K MIPI to DP transmitter designed for portable device.


This is the v16 version, any mistakes, please let me know, I will fix it in
the next series.

Change history:
v16: Fix compile error
 - Fix compiling error of incompitible interface of ".mode_valid()"

v15: Fix comments from Sam and Hsin-Yi Wang
 - Remove connector
 - Allocate memory for edid at ".get_edid()"

v14: Fix comments from Sam and Nicolas
 - Check flags at drm_bridge_attach
 - Use panel_bridge instead of drm_panel
 - Fix not correct return value

v13: Fix comments from Launrent Pinchart and Rob Herring
 - Picked up Rob's Reviewed-By
 - Add .detect and .get_edid interface in bridge funcs.

v12: Fix comments from Hsin-Yi Wang
 - Rebase the code on kernel 5.7, fix DRM interface not match issue.

v11: Fix comments from Rob Herring
 - Update commit message.
 - Remove unused label.

v10: Fix comments from Rob Herring, Daniel.
 - Fix dt_binding_check warning.
 - Update description.

v9: Fix comments from Sam, Nicolas, Daniel
 - Remove extcon interface.
 - Remove DPI support.
 - Fix dt_binding_check complains.
 - Code clean up and update description.

v8: Fix comments from Nicolas.
 - Fix several coding format.
 - Update description.

v7:
 - Fix critical timing(eg:odd hfp/hbp) in "mode_fixup" interface,
   enhance MIPI RX tolerance by setting register MIPI_DIGITAL_ADJ_1 to 0x3D.


Xin Ji (2):
  dt-bindings: drm/bridge: anx7625: MIPI to DP transmitter DT schema
  drm/bridge: anx7625: Add anx7625 MIPI DSI/DPI to DP

 .../bindings/display/bridge/analogix,anx7625.yaml  |   95 +
 drivers/gpu/drm/bridge/analogix/Kconfig|9 +
 drivers/gpu/drm/bridge/analogix/Makefile   |1 +
 drivers/gpu/drm/bridge/analogix/anx7625.c  | 1849 
 drivers/gpu/drm/bridge/analogix/anx7625.h  |  390 +
 5 files changed, 2344 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
 create mode 100644 drivers/gpu/drm/bridge/analogix/anx7625.c
 create mode 100644 drivers/gpu/drm/bridge/analogix/anx7625.h

-- 
2.7.4

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

Re: [RFC PATCH] Add bridge driver to connect sensors to CIO2 device via software nodes on ACPI platforms

2020-09-17 Thread Dan Scally 
Hi Greg - thanks for the comments, appreciate it (sorry there's so many,
I'm new to both C and kernel work)

On 17/09/2020 08:53, Greg KH wrote:
> On Wed, Sep 16, 2020 at 10:36:18PM +0100, Daniel Scally wrote:
>>  MAINTAINERS  |   6 +
>>  drivers/media/pci/intel/ipu3/ipu3-cio2.c |  67 +++-
> staging drivers should be self-contained, and not modify stuff outside
> of drivers/staging/
>
>>  drivers/staging/media/ipu3/Kconfig   |  15 +
>>  drivers/staging/media/ipu3/Makefile  |   1 +
>>  drivers/staging/media/ipu3/cio2-bridge.c | 448 +++
> Why does this have to be in drivers/staging/ at all?  Why not spend the
> time to fix it up properly and get it merged correctly?  It's a very
> small driver, and should be smaller, so it should not be a lot of work
> to do.  And it would be faster to do that than to take it through
> staging...
I was just under the impression that that was the process to be honest,
if that's not right I'll just move it directly to drivers/media/ipu3
>>  5 files changed, 534 insertions(+), 3 deletions(-)
>>  create mode 100644 drivers/staging/media/ipu3/cio2-bridge.c
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index b5cfab015bd6..55b0b9888bc0 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -9152,6 +9152,12 @@ S:Maintained
>>  W:  http://www.adaptec.com/
>>  F:  drivers/scsi/ips*
>>  
>> +IPU3 CIO2 Bridge Driver
>> +M:  Daniel Scally 
>> +L:  linux-me...@vger.kernel.org
>> +S:  Maintained
>> +F:  drivers/staging/media/ipu3/cio2-bridge.c
>> +
>>  IPVS
>>  M:  Wensong Zhang 
>>  M:  Simon Horman 
>> diff --git a/drivers/media/pci/intel/ipu3/ipu3-cio2.c 
>> b/drivers/media/pci/intel/ipu3/ipu3-cio2.c
>> index 92f5eadf2c99..fd941d2c7581 100644
>> --- a/drivers/media/pci/intel/ipu3/ipu3-cio2.c
>> +++ b/drivers/media/pci/intel/ipu3/ipu3-cio2.c
>> @@ -1719,6 +1719,59 @@ static void cio2_queues_exit(struct cio2_device *cio2)
>>  cio2_queue_exit(cio2, >queue[i]);
>>  }
>>  
>> +static int cio2_probe_can_progress(struct pci_dev *pci_dev)
>> +{
>> +void *sensor;
> This is a huge flag that something is wrong, why void?
>
>> +
>> +/*
>> + * On ACPI platforms, we need to probe _after_ sensors wishing to 
>> connect
>> + * to cio2 have added a device link. If there are no consumers yet, then
>> + * we need to defer. The .sync_state() callback will then be called 
>> after
>> + * all linked sensors have probed
>> + */
>> +
>> +if (IS_ENABLED(CONFIG_ACPI)) {
>> +sensor = (struct device *)list_first_entry_or_null(
> And you cast it???  Not right at all.

Yeah sorry; misunderstood entirely how that was supposed to work. From
the following comment, this probably needs re-implementing as
list_for_each_entry anyway

>
>> +
>> _dev->dev.links.consumers,
>> +struct 
>> dev_links_info,
>> +consumers);
> How do you "know" this is the first link?  This feels really really
> wrong and very fragile.
>
>> +
>> +if (!sensor)
>> +return -EPROBE_DEFER;
> So any random value will work?  I doubt it :)
So the intention was just to check that there is _a_ linked device,
which I had been assuming would be a sensor that wanted to use the cio2
device. That's probably not very smart in retrospect; I hadn't
considered other none-me pieces of code linking in. I guess a better
approach would be to check all the linked devices with list_each_entry
and determine if at least one of them is a sensor.
>
>> +}
>> +
>> +return 0;
>> +}
>> +
>> +void cio2_sync_state(struct device *dev)
>> +{
>> +struct cio2_device *cio2;
>> +int ret = 0;
>> +
>> +if (IS_ENABLED(CONFIG_ACPI)) {
>> +cio2 = dev_get_drvdata(dev);
>> +
>> +if (!cio2) {
>> +dev_err(dev, "Failed to retrieve driver data\n");
> How can this fail?
Yeah I guess if the cio2_pci_probe() never made it to setting driver
data then the sync_state() shouldn't get called; thanks.
>
>> +return;
> No error value?
The prototype for sync_state callbacks is to return void, so my
understanding is it can't return an error value.  I guess a better thing
to do might be call another function performing cleanup and unloading
the driver before the return or something along those lines though.
>
>> +}
>> +
>> +/* insert the bridge driver to connect sensors via software 
>> nodes */
>> +ret = request_module("cio2-bridge");
>> +
>> +if (ret)
>> +dev_err(dev, "Failed to insert cio2-bridge\n");
> Yet you keep on in the function???
>> +
>> +ret = cio2_parse_firmware(cio2);
>> +
>> +if (ret) {
>> +v4l2_async_notifier_unregister(>notifier);
>> +

Re: [RFC PATCH] Add bridge driver to connect sensors to CIO2 device via software nodes on ACPI platforms

2020-09-17 Thread Dan Carpenter 
On Wed, Sep 16, 2020 at 10:36:18PM +0100, Daniel Scally wrote:
> diff --git a/drivers/media/pci/intel/ipu3/ipu3-cio2.c 
> b/drivers/media/pci/intel/ipu3/ipu3-cio2.c
> index 92f5eadf2c99..fd941d2c7581 100644
> --- a/drivers/media/pci/intel/ipu3/ipu3-cio2.c
> +++ b/drivers/media/pci/intel/ipu3/ipu3-cio2.c
> @@ -1719,6 +1719,59 @@ static void cio2_queues_exit(struct cio2_device *cio2)
>   cio2_queue_exit(cio2, >queue[i]);
>  }
>  
> +static int cio2_probe_can_progress(struct pci_dev *pci_dev)
> +{
> + void *sensor;
> +
> + /*
> +  * On ACPI platforms, we need to probe _after_ sensors wishing to 
> connect
> +  * to cio2 have added a device link. If there are no consumers yet, then
> +  * we need to defer. The .sync_state() callback will then be called 
> after
> +  * all linked sensors have probed
> +  */
> +
> + if (IS_ENABLED(CONFIG_ACPI)) {

Reverse this condition.

if (!IS_ENABLED(CONFIG_ACPI))
return 0;


> + sensor = (struct device *)list_first_entry_or_null(
> + 
> _dev->dev.links.consumers,
> + struct 
> dev_links_info,
> + consumers);
> +
> + if (!sensor)
> + return -EPROBE_DEFER;

Get rid of the cast.

if (list_empty(_dev->dev.links.consumers))
return -EPROBE_DEFER;

return 0;

> + }
> +
> + return 0;
> +}
> +
> +void cio2_sync_state(struct device *dev)
> +{
> + struct cio2_device *cio2;
> + int ret = 0;

Delete the initialization.

> +
> + if (IS_ENABLED(CONFIG_ACPI)) {

Reverse.

> + cio2 = dev_get_drvdata(dev);
> +
> + if (!cio2) {

Delete the blank line between the call and the test.  They're part of
the same step.  "cio2" can't be NULL anyway, so delete the test.

> + dev_err(dev, "Failed to retrieve driver data\n");
> + return;
> + }
> +
> + /* insert the bridge driver to connect sensors via software 
> nodes */
> + ret = request_module("cio2-bridge");
> +
> + if (ret)
> + dev_err(dev, "Failed to insert cio2-bridge\n");
> +
> + ret = cio2_parse_firmware(cio2);
> +
> + if (ret) {
> + v4l2_async_notifier_unregister(>notifier);
> + v4l2_async_notifier_cleanup(>notifier);
> + cio2_queues_exit(cio2);
> + }
> + }
> +}
> +
>  / PCI interface /
>  
>  static int cio2_pci_config_setup(struct pci_dev *dev)
> @@ -1746,6 +1799,11 @@ static int cio2_pci_probe(struct pci_dev *pci_dev,
>   void __iomem *const *iomap;
>   int r;
>  
> + r = cio2_probe_can_progress(pci_dev);
> +
> + if (r)
> + return -EPROBE_DEFER;
> +
>   cio2 = devm_kzalloc(_dev->dev, sizeof(*cio2), GFP_KERNEL);
>   if (!cio2)
>   return -ENOMEM;
> @@ -1821,9 +1879,11 @@ static int cio2_pci_probe(struct pci_dev *pci_dev,
>   v4l2_async_notifier_init(>notifier);
>  
>   /* Register notifier for subdevices we care */
> - r = cio2_parse_firmware(cio2);
> - if (r)
> - goto fail_clean_notifier;
> + if (!IS_ENABLED(CONFIG_ACPI)) {
> + r = cio2_parse_firmware(cio2);
> + if (r)
> + goto fail_clean_notifier;
> + }
>  
>   r = devm_request_irq(_dev->dev, pci_dev->irq, cio2_irq,
>IRQF_SHARED, CIO2_NAME, cio2);
> @@ -2052,6 +2112,7 @@ static struct pci_driver cio2_pci_driver = {
>   .remove = cio2_pci_remove,
>   .driver = {
>   .pm = _pm_ops,
> + .sync_state = cio2_sync_state
>   },
>  };
>  
> diff --git a/drivers/staging/media/ipu3/Kconfig 
> b/drivers/staging/media/ipu3/Kconfig
> index 3e9640523e50..08842fd8c0da 100644
> --- a/drivers/staging/media/ipu3/Kconfig
> +++ b/drivers/staging/media/ipu3/Kconfig
> @@ -14,3 +14,18 @@ config VIDEO_IPU3_IMGU
>  
> Say Y or M here if you have a Skylake/Kaby Lake SoC with a MIPI
> camera. The module will be called ipu3-imgu.
> +
> +config VIDEO_CIO2_BRIDGE
> + tristate "IPU3 CIO2 Sensor Bridge Driver"
> + depends on PCI && VIDEO_V4L2
> + depends on ACPI
> + depends on X86
> + help
> +   This module provides a bridge connecting sensors (I.E. cameras) to
> +   the CIO2 device infrastructure via software nodes built from 
> information
> +   parsed from the SSDB buffer.
> +
> +   Say Y or M here if your platform's cameras use IPU3 with connections
> +   that should be defined in ACPI. The module will be called cio2-bridge.
> +
> +   If in doubt, say N here.
> \ No newline at end of file
> diff --git a/drivers/staging/media/ipu3/Makefile 
>

Re: [PATCH 1/2] staging: rtl8188eu: use __func__ in hal directory

2020-09-17 Thread Joe Perches 
On Thu, 2020-09-17 at 09:13 +0200, Michael Straube wrote:
> Use __func__ instead of hardcoded function names to clear
> checkpatch warnings.
[]
> diff --git a/drivers/staging/rtl8188eu/hal/odm.c 
> b/drivers/staging/rtl8188eu/hal/odm.c
[]
> @@ -249,7 +249,7 @@ void odm_CommonInfoSelfUpdate(struct odm_dm_struct 
> *pDM_Odm)
>  
>  void odm_CmnInfoInit_Debug(struct odm_dm_struct *pDM_Odm)
>  {
> - ODM_RT_TRACE(pDM_Odm, ODM_COMP_COMMON, ODM_DBG_LOUD, 
> ("odm_CmnInfoInit_Debug==>\n"));
> + ODM_RT_TRACE(pDM_Odm, ODM_COMP_COMMON, ODM_DBG_LOUD, ("%s==>\n", 
> __func__));
>   ODM_RT_TRACE(pDM_Odm, ODM_COMP_COMMON, ODM_DBG_LOUD, 
> ("SupportPlatform=%d\n", pDM_Odm->SupportPlatform));
>   ODM_RT_TRACE(pDM_Odm, ODM_COMP_COMMON, ODM_DBG_LOUD, 
> ("SupportAbility=0x%x\n", pDM_Odm->SupportAbility));
>   ODM_RT_TRACE(pDM_Odm, ODM_COMP_COMMON, ODM_DBG_LOUD, 
> ("SupportInterface=%d\n", pDM_Odm->SupportInterface));

These ODM_RT_TRACE macro uses are rather ugly.
Perhaps a rename to odm_dbg would be better.
So would removing the unnecessary parentheses from
the macro uses and fixing the macro definition

Maybe using a perl script something like the below:

our $balanced_parens = qr/(\((?:[^\(\)]++|(?-1))*\))/;

sub deparenthesize {
my ($string) = @_;
return "" if (!defined($string));

while ($string =~ /^\s*\(.*\)\s*$/s) {
$string =~ s@^\s*\(\s*@@s;
$string =~ s@\s*\)\s*$@@s;
}

return $string;
}

foreach my $file (@ARGV) {
my $FILE;
my $count;
open($FILE, '<', $file) or die("Can't read $file $!\n");
undef $/;
my $text = (<$FILE>);
close($FILE);
$count = $text =~ 
s/ODM_RT_TRACE\s*\(\s*([^,]*),\s*ODM_COMP_(\w+)\s*,\s*ODM_DBG_(\w+)\s*,\s*($balanced_parens)\s*\)\s*;/"odm_dbg($1,
 $2, $3, " . deparenthesize($4) . ");"/ge;
if ($count > 0) {
open($FILE, '>', $file) or die("Can't write $file $!\n");
print $FILE $text;
close($FILE);
}
}


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

[PATCH v16 1/2] dt-bindings: drm/bridge: anx7625: MIPI to DP transmitter DT schema

2020-09-17 Thread Xin Ji 
anx7625: MIPI to DP transmitter DT schema

Signed-off-by: Xin Ji 
Reviewed-by: Rob Herring 
---
 .../bindings/display/bridge/analogix,anx7625.yaml  | 95 ++
 1 file changed, 95 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml

diff --git 
a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml 
b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
new file mode 100644
index 000..60585a4
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
@@ -0,0 +1,95 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+# Copyright 2019 Analogix Semiconductor, Inc.
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/display/bridge/analogix,anx7625.yaml#;
+$schema: "http://devicetree.org/meta-schemas/core.yaml#;
+
+title: Analogix ANX7625 SlimPort (4K Mobile HD Transmitter)
+
+maintainers:
+  - Xin Ji 
+
+description: |
+  The ANX7625 is an ultra-low power 4K Mobile HD Transmitter
+  designed for portable devices.
+
+properties:
+  compatible:
+items:
+  - const: analogix,anx7625
+
+  reg:
+maxItems: 1
+
+  interrupts:
+description: used for interrupt pin B8.
+maxItems: 1
+
+  enable-gpios:
+description: used for power on chip control, POWER_EN pin D2.
+maxItems: 1
+
+  reset-gpios:
+description: used for reset chip control, RESET_N pin B7.
+maxItems: 1
+
+  ports:
+type: object
+
+properties:
+  port@0:
+type: object
+description:
+  Video port for MIPI DSI input.
+
+  port@1:
+type: object
+description:
+  Video port for panel or connector.
+
+required:
+- port@0
+- port@1
+
+required:
+  - compatible
+  - reg
+  - ports
+
+additionalProperties: false
+
+examples:
+  - |
+#include 
+
+i2c0 {
+#address-cells = <1>;
+#size-cells = <0>;
+
+encoder@58 {
+compatible = "analogix,anx7625";
+reg = <0x58>;
+enable-gpios = < 45 GPIO_ACTIVE_HIGH>;
+reset-gpios = < 73 GPIO_ACTIVE_HIGH>;
+
+ports {
+#address-cells = <1>;
+#size-cells = <0>;
+
+mipi2dp_bridge_in: port@0 {
+reg = <0>;
+anx7625_in: endpoint {
+remote-endpoint = <_dsi>;
+};
+};
+
+mipi2dp_bridge_out: port@1 {
+reg = <1>;
+anx7625_out: endpoint {
+remote-endpoint = <_in>;
+};
+};
+};
+};
+};
-- 
2.7.4

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

Re: [PATCH 0/5] ARM: dts: sun8i: r40: Enable video decoder

2020-09-17 Thread Hans Verkuil 
Hi Maxime,

On 27/08/2020 17:19, Maxime Ripard wrote:
> On Tue, Aug 25, 2020 at 07:35:18PM +0200, Jernej Skrabec wrote:
>> Allwinner R40 SoC contains video engine very similar to that in A33.
>>
>> First two patches add system controller nodes and the rest of them
>> add support for Cedrus VPU.
>>
>> Please take a look.
> 
> Applied all 5 patches, thanks

Just to confirm: you've taken patches 3 and 4 as well? If so, then I can mark 
them as
done in patchwork.

Regards,

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

[PATCH v16 2/2] drm/bridge: anx7625: Add anx7625 MIPI DSI/DPI to DP

2020-09-17 Thread Xin Ji 
The ANX7625 is an ultra-low power 4K Mobile HD Transmitter designed
for portable device. It converts MIPI DSI/DPI to DisplayPort 1.3 4K.

Signed-off-by: Xin Ji 
---
 drivers/gpu/drm/bridge/analogix/Kconfig   |9 +
 drivers/gpu/drm/bridge/analogix/Makefile  |1 +
 drivers/gpu/drm/bridge/analogix/anx7625.c | 1849 +
 drivers/gpu/drm/bridge/analogix/anx7625.h |  390 ++
 4 files changed, 2249 insertions(+)
 create mode 100644 drivers/gpu/drm/bridge/analogix/anx7625.c
 create mode 100644 drivers/gpu/drm/bridge/analogix/anx7625.h

diff --git a/drivers/gpu/drm/bridge/analogix/Kconfig 
b/drivers/gpu/drm/bridge/analogix/Kconfig
index e1fa7d8..024ea2a 100644
--- a/drivers/gpu/drm/bridge/analogix/Kconfig
+++ b/drivers/gpu/drm/bridge/analogix/Kconfig
@@ -25,3 +25,12 @@ config DRM_ANALOGIX_ANX78XX
 config DRM_ANALOGIX_DP
tristate
depends on DRM
+
+config DRM_ANALOGIX_ANX7625
+   tristate "Analogix Anx7625 MIPI to DP interface support"
+   depends on DRM
+   depends on OF
+   help
+ ANX7625 is an ultra-low power 4K mobile HD transmitter
+ designed for portable devices. It converts MIPI/DPI to
+ DisplayPort1.3 4K.
diff --git a/drivers/gpu/drm/bridge/analogix/Makefile 
b/drivers/gpu/drm/bridge/analogix/Makefile
index 97669b3..44da392 100644
--- a/drivers/gpu/drm/bridge/analogix/Makefile
+++ b/drivers/gpu/drm/bridge/analogix/Makefile
@@ -1,5 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0-only
 analogix_dp-objs := analogix_dp_core.o analogix_dp_reg.o analogix-i2c-dptx.o
 obj-$(CONFIG_DRM_ANALOGIX_ANX6345) += analogix-anx6345.o
+obj-$(CONFIG_DRM_ANALOGIX_ANX7625) += anx7625.o
 obj-$(CONFIG_DRM_ANALOGIX_ANX78XX) += analogix-anx78xx.o
 obj-$(CONFIG_DRM_ANALOGIX_DP) += analogix_dp.o
diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c 
b/drivers/gpu/drm/bridge/analogix/anx7625.c
new file mode 100644
index 000..cec37da
--- /dev/null
+++ b/drivers/gpu/drm/bridge/analogix/anx7625.c
@@ -0,0 +1,1849 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright(c) 2020, Analogix Semiconductor. All rights reserved.
+ *
+ */
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+
+#include "anx7625.h"
+
+/*
+ * There is a sync issue while access I2C register between AP(CPU) and
+ * internal firmware(OCM), to avoid the race condition, AP should access
+ * the reserved slave address before slave address occurs changes.
+ */
+static int i2c_access_workaround(struct anx7625_data *ctx,
+struct i2c_client *client)
+{
+   u8 offset;
+   struct device *dev = >dev;
+   int ret;
+
+   if (client == ctx->last_client)
+   return 0;
+
+   ctx->last_client = client;
+
+   if (client == ctx->i2c.tcpc_client)
+   offset = RSVD_00_ADDR;
+   else if (client == ctx->i2c.tx_p0_client)
+   offset = RSVD_D1_ADDR;
+   else if (client == ctx->i2c.tx_p1_client)
+   offset = RSVD_60_ADDR;
+   else if (client == ctx->i2c.rx_p0_client)
+   offset = RSVD_39_ADDR;
+   else if (client == ctx->i2c.rx_p1_client)
+   offset = RSVD_7F_ADDR;
+   else
+   offset = RSVD_00_ADDR;
+
+   ret = i2c_smbus_write_byte_data(client, offset, 0x00);
+   if (ret < 0)
+   DRM_DEV_ERROR(dev,
+ "fail to access i2c id=%x\n:%x",
+ client->addr, offset);
+
+   return ret;
+}
+
+static int anx7625_reg_read(struct anx7625_data *ctx,
+   struct i2c_client *client, u8 reg_addr)
+{
+   int ret;
+   struct device *dev = >dev;
+
+   i2c_access_workaround(ctx, client);
+
+   ret = i2c_smbus_read_byte_data(client, reg_addr);
+   if (ret < 0)
+   DRM_DEV_ERROR(dev, "read i2c fail id=%x:%x\n",
+ client->addr, reg_addr);
+
+   return ret;
+}
+
+static int anx7625_reg_block_read(struct anx7625_data *ctx,
+ struct i2c_client *client,
+ u8 reg_addr, u8 len, u8 *buf)
+{
+   int ret;
+   struct device *dev = >dev;
+
+   i2c_access_workaround(ctx, client);
+
+   ret = i2c_smbus_read_i2c_block_data(client, reg_addr, len, buf);
+   if (ret < 0)
+   DRM_DEV_ERROR(dev, "read i2c block fail id=%x:%x\n",
+ client->addr, reg_addr);
+
+   return ret;
+}
+
+static int anx7625_reg_write(struct anx7625_data *ctx,
+struct i2c_client *client,
+u8 reg_addr, u8 reg_val)
+{
+   int ret;
+   struct device *dev = >dev;
+
+   i2c_access_workaround(ctx, client);
+
+   ret =

Re: [RFC PATCH] Add bridge driver to connect sensors to CIO2 device via software nodes on ACPI platforms

2020-09-17 Thread Greg KH 
On Wed, Sep 16, 2020 at 10:36:18PM +0100, Daniel Scally wrote:
>  MAINTAINERS  |   6 +
>  drivers/media/pci/intel/ipu3/ipu3-cio2.c |  67 +++-

staging drivers should be self-contained, and not modify stuff outside
of drivers/staging/

>  drivers/staging/media/ipu3/Kconfig   |  15 +
>  drivers/staging/media/ipu3/Makefile  |   1 +
>  drivers/staging/media/ipu3/cio2-bridge.c | 448 +++

Why does this have to be in drivers/staging/ at all?  Why not spend the
time to fix it up properly and get it merged correctly?  It's a very
small driver, and should be smaller, so it should not be a lot of work
to do.  And it would be faster to do that than to take it through
staging...

>  5 files changed, 534 insertions(+), 3 deletions(-)
>  create mode 100644 drivers/staging/media/ipu3/cio2-bridge.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index b5cfab015bd6..55b0b9888bc0 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -9152,6 +9152,12 @@ S: Maintained
>  W:   http://www.adaptec.com/
>  F:   drivers/scsi/ips*
>  
> +IPU3 CIO2 Bridge Driver
> +M:   Daniel Scally 
> +L:   linux-me...@vger.kernel.org
> +S:   Maintained
> +F:   drivers/staging/media/ipu3/cio2-bridge.c
> +
>  IPVS
>  M:   Wensong Zhang 
>  M:   Simon Horman 
> diff --git a/drivers/media/pci/intel/ipu3/ipu3-cio2.c 
> b/drivers/media/pci/intel/ipu3/ipu3-cio2.c
> index 92f5eadf2c99..fd941d2c7581 100644
> --- a/drivers/media/pci/intel/ipu3/ipu3-cio2.c
> +++ b/drivers/media/pci/intel/ipu3/ipu3-cio2.c
> @@ -1719,6 +1719,59 @@ static void cio2_queues_exit(struct cio2_device *cio2)
>   cio2_queue_exit(cio2, >queue[i]);
>  }
>  
> +static int cio2_probe_can_progress(struct pci_dev *pci_dev)
> +{
> + void *sensor;

This is a huge flag that something is wrong, why void?

> +
> + /*
> +  * On ACPI platforms, we need to probe _after_ sensors wishing to 
> connect
> +  * to cio2 have added a device link. If there are no consumers yet, then
> +  * we need to defer. The .sync_state() callback will then be called 
> after
> +  * all linked sensors have probed
> +  */
> +
> + if (IS_ENABLED(CONFIG_ACPI)) {
> + sensor = (struct device *)list_first_entry_or_null(

And you cast it???  Not right at all.

> + 
> _dev->dev.links.consumers,
> + struct 
> dev_links_info,
> + consumers);

How do you "know" this is the first link?  This feels really really
wrong and very fragile.

> +
> + if (!sensor)
> + return -EPROBE_DEFER;

So any random value will work?  I doubt it :)

> + }
> +
> + return 0;
> +}
> +
> +void cio2_sync_state(struct device *dev)
> +{
> + struct cio2_device *cio2;
> + int ret = 0;
> +
> + if (IS_ENABLED(CONFIG_ACPI)) {
> + cio2 = dev_get_drvdata(dev);
> +
> + if (!cio2) {
> + dev_err(dev, "Failed to retrieve driver data\n");

How can this fail?

> + return;

No error value?

> + }
> +
> + /* insert the bridge driver to connect sensors via software 
> nodes */
> + ret = request_module("cio2-bridge");
> +
> + if (ret)
> + dev_err(dev, "Failed to insert cio2-bridge\n");

Yet you keep on in the function???

> +
> + ret = cio2_parse_firmware(cio2);
> +
> + if (ret) {
> + v4l2_async_notifier_unregister(>notifier);
> + v4l2_async_notifier_cleanup(>notifier);
> + cio2_queues_exit(cio2);

But you clean up after this error?

> + }
> + }

And again, do not tell anyone?

Feels really wrong...

> +}
> +
>  / PCI interface /
>  
>  static int cio2_pci_config_setup(struct pci_dev *dev)
> @@ -1746,6 +1799,11 @@ static int cio2_pci_probe(struct pci_dev *pci_dev,
>   void __iomem *const *iomap;
>   int r;
>  
> + r = cio2_probe_can_progress(pci_dev);
> +
> + if (r)
> + return -EPROBE_DEFER;
> +
>   cio2 = devm_kzalloc(_dev->dev, sizeof(*cio2), GFP_KERNEL);
>   if (!cio2)
>   return -ENOMEM;
> @@ -1821,9 +1879,11 @@ static int cio2_pci_probe(struct pci_dev *pci_dev,
>   v4l2_async_notifier_init(>notifier);
>  
>   /* Register notifier for subdevices we care */
> - r = cio2_parse_firmware(cio2);
> - if (r)
> - goto fail_clean_notifier;
> + if (!IS_ENABLED(CONFIG_ACPI)) {
> + r = cio2_parse_firmware(cio2);
> + if (r)
> + goto fail_clean_notifier;
> + }
>  
>   r = devm_request_irq(_dev->dev, pci_dev->irq, cio2_irq,
>IRQF_SHARED, CIO2_NAME, cio2);
> @@ -2052,6 +2112,7 @@ static struct pci_driver cio2_pci_driver = {
>

[PATCH 1/2] staging: rtl8188eu: use __func__ in hal directory

2020-09-17 Thread Michael Straube 
Use __func__ instead of hardcoded function names to clear
checkpatch warnings.

Signed-off-by: Michael Straube 
---
 drivers/staging/rtl8188eu/hal/hal_intf.c  |  4 +-
 drivers/staging/rtl8188eu/hal/odm.c   | 60 +--
 drivers/staging/rtl8188eu/hal/phy.c   |  2 +-
 drivers/staging/rtl8188eu/hal/pwrseqcmd.c | 25 
 .../staging/rtl8188eu/hal/rtl8188e_hal_init.c |  2 +-
 .../staging/rtl8188eu/hal/rtl8188eu_xmit.c|  2 +-
 drivers/staging/rtl8188eu/hal/usb_halinit.c   |  5 +-
 7 files changed, 50 insertions(+), 50 deletions(-)

diff --git a/drivers/staging/rtl8188eu/hal/hal_intf.c 
b/drivers/staging/rtl8188eu/hal/hal_intf.c
index b8fecc952cfc..9585dffc63a3 100644
--- a/drivers/staging/rtl8188eu/hal/hal_intf.c
+++ b/drivers/staging/rtl8188eu/hal/hal_intf.c
@@ -23,7 +23,7 @@ uint rtw_hal_init(struct adapter *adapt)
rtw_hal_notch_filter(adapt, 1);
} else {
adapt->hw_init_completed = false;
-   DBG_88E("rtw_hal_init: hal__init fail\n");
+   DBG_88E("%s: hal__init fail\n", __func__);
}
 
RT_TRACE(_module_hal_init_c_, _drv_err_,
@@ -41,7 +41,7 @@ uint rtw_hal_deinit(struct adapter *adapt)
if (status == _SUCCESS)
adapt->hw_init_completed = false;
else
-   DBG_88E("\n rtw_hal_deinit: hal_init fail\n");
+   DBG_88E("\n %s: hal_init fail\n", __func__);
 
return status;
 }
diff --git a/drivers/staging/rtl8188eu/hal/odm.c 
b/drivers/staging/rtl8188eu/hal/odm.c
index edc278e5744f..d6c4c7d023d1 100644
--- a/drivers/staging/rtl8188eu/hal/odm.c
+++ b/drivers/staging/rtl8188eu/hal/odm.c
@@ -249,7 +249,7 @@ void odm_CommonInfoSelfUpdate(struct odm_dm_struct *pDM_Odm)
 
 void odm_CmnInfoInit_Debug(struct odm_dm_struct *pDM_Odm)
 {
-   ODM_RT_TRACE(pDM_Odm, ODM_COMP_COMMON, ODM_DBG_LOUD, 
("odm_CmnInfoInit_Debug==>\n"));
+   ODM_RT_TRACE(pDM_Odm, ODM_COMP_COMMON, ODM_DBG_LOUD, ("%s==>\n", 
__func__));
ODM_RT_TRACE(pDM_Odm, ODM_COMP_COMMON, ODM_DBG_LOUD, 
("SupportPlatform=%d\n", pDM_Odm->SupportPlatform));
ODM_RT_TRACE(pDM_Odm, ODM_COMP_COMMON, ODM_DBG_LOUD, 
("SupportAbility=0x%x\n", pDM_Odm->SupportAbility));
ODM_RT_TRACE(pDM_Odm, ODM_COMP_COMMON, ODM_DBG_LOUD, 
("SupportInterface=%d\n", pDM_Odm->SupportInterface));
@@ -267,7 +267,7 @@ void odm_CmnInfoInit_Debug(struct odm_dm_struct *pDM_Odm)
 
 void odm_CmnInfoHook_Debug(struct odm_dm_struct *pDM_Odm)
 {
-   ODM_RT_TRACE(pDM_Odm, ODM_COMP_COMMON, ODM_DBG_LOUD, 
("odm_CmnInfoHook_Debug==>\n"));
+   ODM_RT_TRACE(pDM_Odm, ODM_COMP_COMMON, ODM_DBG_LOUD, ("%s==>\n", 
__func__));
ODM_RT_TRACE(pDM_Odm, ODM_COMP_COMMON, ODM_DBG_LOUD, 
("pNumTxBytesUnicast=%llu\n", *pDM_Odm->pNumTxBytesUnicast));
ODM_RT_TRACE(pDM_Odm, ODM_COMP_COMMON, ODM_DBG_LOUD, 
("pNumRxBytesUnicast=%llu\n", *pDM_Odm->pNumRxBytesUnicast));
ODM_RT_TRACE(pDM_Odm, ODM_COMP_COMMON, ODM_DBG_LOUD, 
("pWirelessMode=0x%x\n", *pDM_Odm->pWirelessMode));
@@ -282,7 +282,7 @@ void odm_CmnInfoHook_Debug(struct odm_dm_struct *pDM_Odm)
 
 void odm_CmnInfoUpdate_Debug(struct odm_dm_struct *pDM_Odm)
 {
-   ODM_RT_TRACE(pDM_Odm, ODM_COMP_COMMON, ODM_DBG_LOUD, 
("odm_CmnInfoUpdate_Debug==>\n"));
+   ODM_RT_TRACE(pDM_Odm, ODM_COMP_COMMON, ODM_DBG_LOUD, ("%s==>\n", 
__func__));
ODM_RT_TRACE(pDM_Odm, ODM_COMP_COMMON, ODM_DBG_LOUD, 
("bWIFI_Direct=%d\n", pDM_Odm->bWIFI_Direct));
ODM_RT_TRACE(pDM_Odm, ODM_COMP_COMMON, ODM_DBG_LOUD, 
("bWIFI_Display=%d\n", pDM_Odm->bWIFI_Display));
ODM_RT_TRACE(pDM_Odm, ODM_COMP_COMMON, ODM_DBG_LOUD, ("bLinked=%d\n", 
pDM_Odm->bLinked));
@@ -339,21 +339,21 @@ void odm_DIG(struct odm_dm_struct *pDM_Odm)
u8 dm_dig_max, dm_dig_min;
u8 CurrentIGI = pDM_DigTable->CurIGValue;
 
-   ODM_RT_TRACE(pDM_Odm, ODM_COMP_DIG, ODM_DBG_LOUD, ("odm_DIG()==>\n"));
+   ODM_RT_TRACE(pDM_Odm, ODM_COMP_DIG, ODM_DBG_LOUD, ("%s()==>\n", 
__func__));
if ((!(pDM_Odm->SupportAbility & ODM_BB_DIG)) || 
(!(pDM_Odm->SupportAbility & ODM_BB_FA_CNT))) {
ODM_RT_TRACE(pDM_Odm, ODM_COMP_DIG, ODM_DBG_LOUD,
-("odm_DIG() Return: SupportAbility ODM_BB_DIG or 
ODM_BB_FA_CNT is disabled\n"));
+("%s() Return: SupportAbility ODM_BB_DIG or 
ODM_BB_FA_CNT is disabled\n", __func__));
return;
}
 
if (*pDM_Odm->pbScanInProcess) {
-   ODM_RT_TRACE(pDM_Odm, ODM_COMP_DIG, ODM_DBG_LOUD, ("odm_DIG() 
Return: In Scan Progress\n"));
+   ODM_RT_TRACE(pDM_Odm, ODM_COMP_DIG, ODM_DBG_LOUD, ("%s() 
Return: In Scan Progress\n", __func__));
return;
}
 
/* add by Neil Chen to avoid PSD is processing */
if (!pDM_Odm->bDMInitialGainEnable) {
-   ODM_RT_TRACE(pDM_Odm, ODM_COMP_DIG, ODM_DBG_LOUD, ("odm_DIG() 
Return: PSD is Processing\n"));
+

[PATCH 2/2] staging: rtl8188eu: clean up comparsion style issues

2020-09-17 Thread Michael Straube 
Move constants to the right side of comparsions to follow kernel
coding style and clear checkpatch warnings. In case of comparsion
to _FAIL we can use '!' since _FAIL is defined as '0'.

WARNING: Comparisons should place the constant on the right side of the test

Signed-off-by: Michael Straube 
---
 drivers/staging/rtl8188eu/core/rtw_mlme_ext.c |  6 +++---
 drivers/staging/rtl8188eu/hal/odm.c   |  4 ++--
 drivers/staging/rtl8188eu/hal/rtl8188e_hal_init.c |  2 +-
 drivers/staging/rtl8188eu/include/rtw_mlme.h  |  4 ++--
 drivers/staging/rtl8188eu/include/wifi.h  |  2 +-
 drivers/staging/rtl8188eu/os_dep/ioctl_linux.c| 10 +-
 6 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c 
b/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c
index dad007f78d8c..4df790c83d9f 100644
--- a/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c
+++ b/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c
@@ -2981,7 +2981,7 @@ static unsigned int OnAssocReq(struct adapter *padapter,
status = _STATS_FAILURE_;
}
 
-   if (_STATS_SUCCESSFUL_ != status)
+   if (status != _STATS_SUCCESSFUL_)
goto OnAssocReqFail;
 
/*  check if the supported rate is ok */
@@ -3072,7 +3072,7 @@ static unsigned int OnAssocReq(struct adapter *padapter,
wpa_ie_len = 0;
}
 
-   if (_STATS_SUCCESSFUL_ != status)
+   if (status != _STATS_SUCCESSFUL_)
goto OnAssocReqFail;
 
pstat->flags &= ~(WLAN_STA_WPS | WLAN_STA_MAYBE_WPS);
@@ -3282,7 +3282,7 @@ static unsigned int OnAssocReq(struct adapter *padapter,
spin_unlock_bh(>asoc_list_lock);
 
/*  now the station is qualified to join our BSS... */
-   if ((pstat->state & WIFI_FW_ASSOC_SUCCESS) && (_STATS_SUCCESSFUL_ == 
status)) {
+   if ((pstat->state & WIFI_FW_ASSOC_SUCCESS) && (status == 
_STATS_SUCCESSFUL_)) {
/* 1 bss_cap_update & sta_info_update */
bss_cap_update_on_sta_join(padapter, pstat);
sta_info_update(padapter, pstat);
diff --git a/drivers/staging/rtl8188eu/hal/odm.c 
b/drivers/staging/rtl8188eu/hal/odm.c
index d6c4c7d023d1..4d659a812aed 100644
--- a/drivers/staging/rtl8188eu/hal/odm.c
+++ b/drivers/staging/rtl8188eu/hal/odm.c
@@ -829,9 +829,9 @@ bool ODM_RAStateCheck(struct odm_dm_struct *pDM_Odm, s32 
RSSI, bool bForceUpdate
}
 
/*  Decide RATRState by RSSI. */
-   if (RSSI > HighRSSIThreshForRA)
+   if (HighRSSIThreshForRA < RSSI)
RATRState = DM_RATR_STA_HIGH;
-   else if (RSSI > LowRSSIThreshForRA)
+   else if (LowRSSIThreshForRA < RSSI)
RATRState = DM_RATR_STA_MIDDLE;
else
RATRState = DM_RATR_STA_LOW;
diff --git a/drivers/staging/rtl8188eu/hal/rtl8188e_hal_init.c 
b/drivers/staging/rtl8188eu/hal/rtl8188e_hal_init.c
index e640c2256ab9..6e131202fde5 100644
--- a/drivers/staging/rtl8188eu/hal/rtl8188e_hal_init.c
+++ b/drivers/staging/rtl8188eu/hal/rtl8188e_hal_init.c
@@ -187,7 +187,7 @@ static s32 _LLTWrite(struct adapter *padapter, u32 address, 
u32 data)
/* polling */
do {
value = usb_read32(padapter, LLTReg);
-   if (_LLT_NO_ACTIVE == _LLT_OP_VALUE(value))
+   if (_LLT_OP_VALUE(value) == _LLT_NO_ACTIVE)
break;
 
if (count > POLLING_LLT_THRESHOLD) {
diff --git a/drivers/staging/rtl8188eu/include/rtw_mlme.h 
b/drivers/staging/rtl8188eu/include/rtw_mlme.h
index 010f0c42368a..1b74b32b8a81 100644
--- a/drivers/staging/rtl8188eu/include/rtw_mlme.h
+++ b/drivers/staging/rtl8188eu/include/rtw_mlme.h
@@ -266,7 +266,7 @@ static inline void set_fwstate(struct mlme_priv *pmlmepriv, 
int state)
 {
pmlmepriv->fw_state |= state;
/* FOR HW integration */
-   if (_FW_UNDER_SURVEY == state)
+   if (state == _FW_UNDER_SURVEY)
pmlmepriv->bScanInProcess = true;
 }
 
@@ -274,7 +274,7 @@ static inline void _clr_fwstate_(struct mlme_priv 
*pmlmepriv, int state)
 {
pmlmepriv->fw_state &= ~state;
/* FOR HW integration */
-   if (_FW_UNDER_SURVEY == state)
+   if (state == _FW_UNDER_SURVEY)
pmlmepriv->bScanInProcess = false;
 }
 
diff --git a/drivers/staging/rtl8188eu/include/wifi.h 
b/drivers/staging/rtl8188eu/include/wifi.h
index 118e215dd6b1..a549b6d0159a 100644
--- a/drivers/staging/rtl8188eu/include/wifi.h
+++ b/drivers/staging/rtl8188eu/include/wifi.h
@@ -326,7 +326,7 @@ static inline unsigned char *get_hdr_bssid(unsigned char 
*pframe)
 
 static inline int IsFrameTypeCtrl(unsigned char *pframe)
 {
-   if (WIFI_CTRL_TYPE == GetFrameType(pframe))
+   if (GetFrameType(pframe) == WIFI_CTRL_TYPE)
return true;
else
return false;
diff --git a/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c 
b/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c
index