Re: [PATCH 0/4] staging: mt7621-pci: Handle minor issues
Hi Sergio, On 25/6/19 10:47 pm, Sergio Paracuellos wrote: On Tue, Jun 25, 2019 at 7:18 AM Sergio Paracuellos wrote: On Tue, Jun 25, 2019 at 7:10 AM Greg Ungerer wrote: On 23/6/19 3:58 pm, Sergio Paracuellos wrote: On Sun, Jun 23, 2019 at 4:15 AM Brett Neumeier wrote: On Fri, Jun 21, 2019 at 7:35 AM Greg Ungerer wrote: On 21/6/19 4:15 pm, Sergio Paracuellos wrote: This patch series properly handle minor issues in this driver. These are: * Disable pcie port clock on pci dirver instead of doing it in the phy driver. The pci driver is the correct place to do this. * Add a missing call to phy_exit function to properly handle the function 'mt7621_pcie_init_port' error path. * Move driver to init in a later stage using 'module_init' instead of using 'arch_initcall'. Patches are only compile-tested. It would be awasome to be tested before applied them (mainly the change to 'module_init' stuff). I have similar results to Greg -- on GnuBee PC1 and PC2, six boot attempts each on a kernel built from staging-next, I have four hangs and eight successful boots. The hanging patterns are similar to previous results. If the full boot logs would be helpful let me know, I can provide them. Thanks for letting me know. One thing we can try is check init order in v4.20 kernel. Can you please try to compile pci driver for the kernel v4.20 tag changing driver's last line 'arch_initcall' into 'module_init'? Just to know if at that working driver putting all the stuff in a later stage stills work as expected. Full dmesg's of this v4.20 wih the change would be helpful. Not exactly what you asked for, but just as useful I think. I have a linux-5.1.14 kernel with the linux-4.20 pci-mt7621.c driver in place. That works great, never hangs, always probes PCI and works. If I change the pci-mt7621.c arch_initcall() to module_init(), then I see the PCI probe happen much latter in boot. Running this I have booted about 15 times, no hangs, PCI bus probed and working on every boot. Perfect. That is exactly what I wanted to know. Thanks for testing this. Boot log below. Regards Greg Can you please test the following change? diff --git a/drivers/staging/mt7621-pci/pci-mt7621.c b/drivers/staging/mt7621-pci/pci-mt7621.c index a981f4f0ed03..9ae14f722c91 100644 --- a/drivers/staging/mt7621-pci/pci-mt7621.c +++ b/drivers/staging/mt7621-pci/pci-mt7621.c @@ -41,8 +41,8 @@ /* MediaTek specific configuration registers */ #define PCIE_FTS_NUM 0x70c -#define PCIE_FTS_NUM_MASK GENMASK(15, 8) -#define PCIE_FTS_NUM_L0(x) ((x) & 0xff << 8) +#define PCIE_FTS_NUM_MASK GENMASK(7, 0) +#define PCIE_FTS_NUM_L0(x) ((x) << 8) /* rt_sysc_membase relative registers */ #define RALINK_CLKCFG1 0x30 @@ -540,7 +540,7 @@ static void mt7621_pcie_enable_ports(struct mt7621_pcie *pcie) write_config(pcie, slot, PCI_COMMAND, val); /* configure RC FTS number to 250 when it leaves L0s */ val = read_config(pcie, slot, PCIE_FTS_NUM); - val &= ~PCIE_FTS_NUM_MASK; + val &= ~(PCIE_FTS_NUM_MASK) << 8; val |= PCIE_FTS_NUM_L0(0x50); write_config(pcie, slot, PCIE_FTS_NUM, val); } Same result. Occasional hangs on boot, sometimes it boots all the way up successfully. But looking at the patch doesn't it do essentially the same thing? Regards Greg Best regards, Sergio Paracuellos Linux version 5.1.14 (gerg@goober) (gcc version 5.4.0 (GCC)) #2 SMP Tue Jun 25 14:34:27 AEST 2019 SoC Type: MediaTek MT7621 ver:1 eco:3 printk: bootconsole [early0] enabled CPU0 revision is: 0001992f (MIPS 1004Kc) OF: fdt: No chosen node found, continuing without MIPS: machine is Digi EX15 Determined physical RAM map: memory: 1000 @ (usable) Initrd not found or empty - disabling initrd VPE topology {2,2} total 4 Primary instruction cache 32kB, VIPT, 4-way, linesize 32 bytes. Primary data cache 32kB, 4-way, PIPT, no aliases, linesize 32 bytes MIPS secondary cache 256kB, 8-way, linesize 32 bytes. Zone ranges: Normal [mem 0x-0x0fff] Movable zone start for each node Early memory node ranges node 0: [mem 0x-0x0fff] Initmem setup node 0 [mem 0x-0x0fff] random: get_random_bytes called from start_kernel+0xb0/0x51c with crng_init=0 percpu: Embedded 15 pages/cpu s30144 r8192 d23104 u61440 Built 1 zonelists, mobility grouping on. Total pages: 65024 Kernel command line: console=ttyS0,115200 ubi.mtd=3 root=/dev/mtdblock5 bootpart=a Dentry cache hash table entries: 32768 (order: 5, 131072 bytes) Inode-cache hash table entries: 16384 (order: 4, 65536 bytes) Writing ErrCtl register=00030517 Readback ErrCtl register=00030517 Memory: 251176K/262144K available (6464K kernel code, 243K rwdata, 1200K rodata, 272K init, 228K bss, 10968K reserved, 0K
[PATCH RESEND] staging: erofs: return the error value if fill_inline_data() fails
From: Yue Hu We should consider the error returned by fill_inline_data() when filling last page in fill_inode(). If not getting inode will be successful even though last page is bad. That is illogical. Also change -EAGAIN to 0 in fill_inline_data() to stand for successful filling. Signed-off-by: Yue Hu --- drivers/staging/erofs/inode.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/staging/erofs/inode.c b/drivers/staging/erofs/inode.c index d6e1e16..1433f25 100644 --- a/drivers/staging/erofs/inode.c +++ b/drivers/staging/erofs/inode.c @@ -156,7 +156,7 @@ static int fill_inline_data(struct inode *inode, void *data, inode->i_link = lnk; set_inode_fast_symlink(inode); } - return -EAGAIN; + return 0; } static int fill_inode(struct inode *inode, int isdir) @@ -223,7 +223,7 @@ static int fill_inode(struct inode *inode, int isdir) inode->i_mapping->a_ops = _raw_access_aops; /* fill last page if inline data is available */ - fill_inline_data(inode, data, ofs); + err = fill_inline_data(inode, data, ofs); } out_unlock: -- 1.9.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH RESEND] staging: erofs: remove unsupported ->datamode check in fill_inline_data()
From: Yue Hu Already check if ->datamode is supported in read_inode(), no need to check again in the next fill_inline_data() only called by fill_inode(). Signed-off-by: Yue Hu --- drivers/staging/erofs/inode.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/staging/erofs/inode.c b/drivers/staging/erofs/inode.c index e51348f..d6e1e16 100644 --- a/drivers/staging/erofs/inode.c +++ b/drivers/staging/erofs/inode.c @@ -129,8 +129,6 @@ static int fill_inline_data(struct inode *inode, void *data, struct erofs_sb_info *sbi = EROFS_I_SB(inode); const int mode = vi->datamode; - DBG_BUGON(mode >= EROFS_INODE_LAYOUT_MAX); - /* should be inode inline C */ if (mode != EROFS_INODE_LAYOUT_INLINE) return 0; -- 1.9.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH V5 10/16] s390: zfcp_fc: use sg helper to operate scatterlist
Hi Steffen, On Tue, Jun 25, 2019 at 12:51:04PM +0200, Steffen Maier wrote: > Hi Ming, > > I don't mind doing this change for zfcp. However, I'm having doubts > regarding the rationale in the commit description. If I understood your > patch series correctly from its cover letter (would have been nice to copy > the SCSI MQ detail part of its core statement in (one of) the patches so it > would make its way into git log), you plan to make a change to scatterlist > allocations *in SCSI MQ*. > > All zfcp changes in this patch set only refer to zfcp-internal remote port > discovery, i.e. they neither come from SCSI (internally) nor from block(mq). If you are sure about that, it is fine to not change zfcp. However, I still suggest to do it because it will make us to audit SCSI chained sg uses much easier. And the change shouldn't have performance effect. > > zfcp_fc.h: > /** > * struct zfcp_fc_req - Container for FC ELS and CT requests sent from zfcp > * @ct_els: data required for issuing fsf command > * @sg_req: scatterlist entry for request data, refers to embedded @u > submember > * @sg_rsp: scatterlist entry for response data, refers to embedded @u > submember > * @u: request and response specific data > > * @u.gpn_ft: GPN_FT specific data > * @u.gpn_ft.sg_rsp2: GPN_FT response, not embedded here, allocated elsewhere > * @u.gpn_ft.req: GPN_FT request > > */ > struct zfcp_fc_req { > struct zfcp_fsf_ct_els ct_els; > struct scatterlist sg_req; > struct scatterlist sg_rsp; > union { > > struct { > struct scatterlist sg_rsp2[ZFCP_FC_GPN_FT_NUM_BUFS - 1]; > struct zfcp_fc_gpn_ft_req req; > } gpn_ft; > > } u; > }; > > So this should be guaranteed to be a linear unchained scatterlist, > independently of your SCSI (or block) changes. > Note: Only remote port discovery also uses u.gpn_ft.sg_rsp2 instead of just > sg_rsp. > zfcp_fc_scan_ports(work) > zfcp_fc_alloc_sg_env(buf_num) >zfcp_fc_sg_setup_table(_req->sg_rsp, buf_num) > (In fact it's somewhat intricate, because it actually uses sg_rsp and seems > to rely on the fact that the subsequent sg_rsp2[] gives enough contiguous > memory to hold buf_num linear scatterlist entries starting with field offset > sg_rsp.) > The other cases use single element and thus linear unchained scatterlist > with sg_rsp (and all cases use sg_req): > Finding symbol: sg_init_one > Database directory: /home/maier/docs/zfcp/tuxmaker/linux/drivers/s390/scsi/ > --- > *** zfcp_dbf.c: > zfcp_dbf_san_in_els[601] sg_init_one(, srb->payload.data, length); > // above tracing part is unrelated to all of scsi/block/zfcp-internal-ct/els > *** zfcp_fc.c: > zfcp_fc_ns_gid_pn_request[388] sg_init_one(_req->sg_req, gid_pn_req, > sizeof(*gid_pn_req)); > zfcp_fc_ns_gid_pn_request[389] sg_init_one(_req->sg_rsp, gid_pn_rsp, > sizeof(*gid_pn_rsp)); > zfcp_fc_adisc[546] sg_init_one(_req->sg_req, > _req->u.adisc.req, > zfcp_fc_adisc[548] sg_init_one(_req->sg_rsp, > _req->u.adisc.rsp, > zfcp_fc_alloc_sg_env[668] sg_init_one(_req->sg_req, > _req->u.gpn_ft.req, > zfcp_fc_gspn[841] sg_init_one(_req->sg_req, gspn_req, > sizeof(*gspn_req)); > zfcp_fc_gspn[842] sg_init_one(_req->sg_rsp, gspn_rsp, > sizeof(*gspn_rsp)); > zfcp_fc_rspn[889] sg_init_one(_req->sg_req, rspn_req, > sizeof(*rspn_req)); > zfcp_fc_rspn[890] sg_init_one(_req->sg_rsp, rspn_rsp, > sizeof(*rspn_rsp)); > --- > > I/O requests from SCSI (MQ) coming through queuecommand have already been > safe for non-linear chained scatterlists in zfcp: > > zfcp_scsi_queuecommand() > zfcp_fsf_fcp_cmnd() >zfcp_qdio_sbals_from_sg(qdio, >qdio_req, scsi_sglist(scsi_cmnd)) > for (; sg; sg = sg_next(sg)) { > > I/O requests from the block layer coming through BSG should also have > already been safe for non-linear chained scatterlists in zfcp: > > zfcp_fc_exec_bsg_job() > zfcp_fc_exec_els_job() >zfcp_fsf_send_els() > zfcp_fsf_setup_ct_els() > zfcp_fsf_setup_ct_els_sbals(req, sg_req, sg_resp) > //depending on hardware features, translate sg into HW control blocks > if (zfcp_adapter_multi_buffer_active()) > zfcp_qdio_sbals_from_sg() //for req, see above > return 0 > /* use single, unchained SBAL if it can hold the request */ > if (zfcp_qdio_sg_one_sbale(sg_req) && zfcp_qdio_sg_one_sbale(sg_resp)) > zfcp_fsf_setup_ct_els_unchained() //single element for req > return 0 > if (!(feat & FSF_FEATURE_ELS_CT_CHAINED_SBALS)) > return -EOPNOTSUPP; >zfcp_qdio_sbals_from_sg() for req, see above
[PATCH 2/2] staging: rtl8723bs: hal: hal_btcoex: Remove unneeded variable PHalData
pHalData is not being used in halbtcoutsrc_LeaveLowPower. So remove the same. Signed-off-by: Hariprasad Kelam --- drivers/staging/rtl8723bs/hal/hal_btcoex.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/staging/rtl8723bs/hal/hal_btcoex.c b/drivers/staging/rtl8723bs/hal/hal_btcoex.c index 99e0b91..0c2a754 100644 --- a/drivers/staging/rtl8723bs/hal/hal_btcoex.c +++ b/drivers/staging/rtl8723bs/hal/hal_btcoex.c @@ -195,7 +195,6 @@ static void halbtcoutsrc_NormalLps(PBTC_COEXIST pBtCoexist) static void halbtcoutsrc_LeaveLowPower(PBTC_COEXIST pBtCoexist) { struct adapter *padapter; - struct hal_com_data *pHalData; s32 ready; unsigned long stime; unsigned long utime; @@ -203,7 +202,6 @@ static void halbtcoutsrc_LeaveLowPower(PBTC_COEXIST pBtCoexist) padapter = pBtCoexist->Adapter; - pHalData = GET_HAL_DATA(padapter); ready = _FAIL; #ifdef LPS_RPWM_WAIT_MS timeout = LPS_RPWM_WAIT_MS; -- 2.7.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/2] staging: rtl8723bs: hal: hal_btcoex: Using comparison to true is error prone
fix below issues reported by checkpatch CHECK: Using comparison to true is error prone CHECK: Using comparison to false is error prone Signed-off-by: Hariprasad Kelam --- drivers/staging/rtl8723bs/hal/hal_btcoex.c | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/staging/rtl8723bs/hal/hal_btcoex.c b/drivers/staging/rtl8723bs/hal/hal_btcoex.c index 66caf34..99e0b91 100644 --- a/drivers/staging/rtl8723bs/hal/hal_btcoex.c +++ b/drivers/staging/rtl8723bs/hal/hal_btcoex.c @@ -290,7 +290,7 @@ static u8 halbtcoutsrc_IsWifiBusy(struct adapter *padapter) if (check_fwstate(pmlmepriv, WIFI_ASOC_STATE) == true) { if (check_fwstate(pmlmepriv, WIFI_AP_STATE) == true) return true; - if (true == pmlmepriv->LinkDetectInfo.bBusyTraffic) + if (pmlmepriv->LinkDetectInfo.bBusyTraffic) return true; } @@ -310,12 +310,12 @@ static u32 _halbtcoutsrc_GetWifiLinkStatus(struct adapter *padapter) if (check_fwstate(pmlmepriv, WIFI_ASOC_STATE) == true) { if (check_fwstate(pmlmepriv, WIFI_AP_STATE) == true) { - if (true == bp2p) + if (bp2p) portConnectedStatus |= WIFI_P2P_GO_CONNECTED; else portConnectedStatus |= WIFI_AP_CONNECTED; } else { - if (true == bp2p) + if (bp2p) portConnectedStatus |= WIFI_P2P_GC_CONNECTED; else portConnectedStatus |= WIFI_STA_CONNECTED; @@ -372,7 +372,7 @@ static u8 halbtcoutsrc_GetWifiScanAPNum(struct adapter *padapter) pmlmeext = >mlmeextpriv; - if (GLBtcWiFiInScanState == false) { + if (!GLBtcWiFiInScanState) { if (pmlmeext->sitesurvey_res.bss_cnt > 0xFF) scan_AP_num = 0xFF; else @@ -1444,7 +1444,7 @@ void hal_btcoex_IQKNotify(struct adapter *padapter, u8 state) void hal_btcoex_BtInfoNotify(struct adapter *padapter, u8 length, u8 *tmpBuf) { - if (GLBtcWiFiInIQKState == true) + if (GLBtcWiFiInIQKState) return; EXhalbtcoutsrc_BtInfoNotify(, tmpBuf, length); -- 2.7.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Call For Confirmation.
Dear winner, Your email address has won you One Million Euro, from Online Lotto, all the E-mail addresses were selected from a data base of internet e-mail users, from which your e-mail address came out as the winning coupon. Winning expiring date 31st of July , 2019. Contact our fiduciary Agent below with your winning number: OL/456/050/006. For immediate release of your cash prize to you, please kindly contact ( Charterd Finance B.V.Netherlands). Send them the following: (i). Your names, (ii) Contact telephone and fax numbers (iii) Contact Address (iv) your winning numbers (v) Quote amount won. Online Lotto Agency. Mrs. Charlotte De Vries. Director of winning claim department. TEL: +31 612 993 300 E-MAIL:charterdfi...@aol.com ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
.......................
Dear Friend I am sorry to encroach into your privacy in this manner my name is Morgan Johnson an American soldier serving in Afghanistan my reason for contacting you is that i would want to confide in you that i have in my possession the sum of $4,980,000.00USD (Four million Nine hundred and eighty thousand American Dollars)the above sum was given to me as my share after a raid and to conceal this amount of money became a problem for me. I am ready to compensate you with 40% of the funds the only thing i require from you is for you to help me receive this funds once i move the money out of Afghanistan because i am in a war zone. I have already mapped out strategy to move the money out of Afghanistan to your location with the aid of the diplomat and the diplomat certainly does not know the real contents of the consignments and he believes that it belongs to a Late Franco-American soldier Spc.Aguila Francisco Xavier who was attacked and killed by enemy with small arms fire and before giving up he trusted me to hand over the package to his family. With my present arrangement/procedure to evacuate this money out of here i am very positive that within two weeks it will get to your location. I shall provide you with the full details as soon as i receive your response. Right now i am very careful with the way i communicate so as to reduce any kind of risk until this money is finally in your custody. I will be communicating with you through email alone because our phone conversation might be monitored by unit bugs,i am doing this on trust and so i would want you to put aside any act of greed as we have a lot to gain in this project.Can I trust you? When you receive this message please kindly send me an e-mail signifying your interest. Respectfully, Captain Morgan Johnson ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 3/4] staging: kpc2000: remove unnecessary parentheses in kpc2000_spi.c
On Tue, Jun 25, 2019 at 10:41:29AM +0200, Simon Sandström wrote: > Fixes checkpatch "CHECK: Unnecessary parentheses around '...'". > > Signed-off-by: Simon Sandström > --- > drivers/staging/kpc2000/kpc2000_spi.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/staging/kpc2000/kpc2000_spi.c > b/drivers/staging/kpc2000/kpc2000_spi.c > index 98484fbb9d2e..68b049f9ad69 100644 > --- a/drivers/staging/kpc2000/kpc2000_spi.c > +++ b/drivers/staging/kpc2000/kpc2000_spi.c > @@ -164,7 +164,7 @@ kp_spi_read_reg(struct kp_spi_controller_state *cs, int > idx) > u64 val; > > addr += idx; > - if ((idx == KP_SPI_REG_CONFIG) && (cs->conf_cache >= 0)) { > + if (idx == KP_SPI_REG_CONFIG && cs->conf_cache >= 0) { Ick, no. I don't want to have to remember the priority order of operations. Leave this as-is, I HATE this checkpatch message. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/2] drivers/staging/rtl8192u: adjust block comments
On Mon, Jun 24, 2019 at 11:46:39AM +0200, Christian Müller wrote: > As stated in coding-styles.rst multiline comments should be structured in a > way, > that the actual comment starts on the second line of the commented portion. > E.g: You sent 2 patches that did different things, yet have the identical subject line :( Please fix up and resend the series. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v4 5/7] lib/hexdump.c: Allow multiple groups to be separated by lines '|'
On Mon, 2019-06-24 at 22:37 -0700, Joe Perches wrote: > On Tue, 2019-06-25 at 13:17 +1000, Alastair D'Silva wrote: > > From: Alastair D'Silva > > > > With the wider display format, it can become hard to identify how > > many > > bytes into the line you are looking at. > > > > The patch adds new flags to hex_dump_to_buffer() and > > print_hex_dump() to > > print vertical lines to separate every N groups of bytes. > > > > eg. > > buf:: 454d414e 43415053|4e495f45 > > 00584544 NAMESPAC|E_INDEX. > > buf:0010: 0002| > > | > > > > Signed-off-by: Alastair D'Silva > > --- > > include/linux/printk.h | 3 +++ > > lib/hexdump.c | 59 > > -- > > 2 files changed, 54 insertions(+), 8 deletions(-) > > > > diff --git a/include/linux/printk.h b/include/linux/printk.h > [] > > @@ -485,6 +485,9 @@ enum { > > > > #define HEXDUMP_ASCII BIT(0) > > #define HEXDUMP_SUPPRESS_REPEATED BIT(1) > > +#define HEXDUMP_2_GRP_LINESBIT(2) > > +#define HEXDUMP_4_GRP_LINESBIT(3) > > +#define HEXDUMP_8_GRP_LINESBIT(4) > > These aren't really bits as only one value should be set > as 8 overrides 4 and 4 overrides 2. This should be the other way around, as we should be emitting alternate seperators based on the smallest grouping (2 implies 4 and 8, and 4 implies 8). I'll fix the logic. I can't come up with a better way to represent these without making the API more complex, if you have a suggestion, I'm happy to hear it. > > I would also expect this to be a value of 2 in your above > example, rather than 8. It's described as groups not bytes. > > The example is showing a what would normally be a space ' ' > separator as a vertical bar '|' every 2nd grouping. > The above example shows a group size of 4 bytes, and HEXDUMP_2_GRP_LINES set, with 2 groups being 8 bytes. I'll make that clearer in the commit message. -- Alastair D'Silva mob: 0423 762 819 skype: alastair_dsilva Twitter: @EvilDeece blog: http://alastair.d-silva.org ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v4 4/7] lib/hexdump.c: Replace ascii bool in hex_dump_to_buffer with flags
On Mon, 2019-06-24 at 22:19 -0700, Joe Perches wrote: > On Tue, 2019-06-25 at 15:06 +1000, Alastair D'Silva wrote: > > The change actions Jani's suggestion: > > https://lkml.org/lkml/2019/6/20/343 > > I suggest not changing any of the existing uses of > hex_dump_to_buffer and only use hex_dump_to_buffer_ext > when necessary for your extended use cases. > > I disagree, adding a wrapper for the benefit of avoiding touching a handful of call sites that are easily amended would be adding technical debt. -- Alastair D'Silva mob: 0423 762 819 skype: alastair_dsilva Twitter: @EvilDeece blog: http://alastair.d-silva.org ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v4 4/7] lib/hexdump.c: Replace ascii bool in hex_dump_to_buffer with flags
On Mon, 2019-06-24 at 22:01 -0700, Joe Perches wrote: > On Tue, 2019-06-25 at 13:17 +1000, Alastair D'Silva wrote: > > From: Alastair D'Silva > > > > In order to support additional features, rename hex_dump_to_buffer > > to > > hex_dump_to_buffer_ext, and replace the ascii bool parameter with > > flags. > [] > > diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c > > b/drivers/gpu/drm/i915/intel_engine_cs.c > [] > > @@ -1338,9 +1338,8 @@ static void hexdump(struct drm_printer *m, > > const void *buf, size_t len) > > } > > > > WARN_ON_ONCE(hex_dump_to_buffer(buf + pos, len - pos, > > - rowsize, sizeof(u32), > > - line, sizeof(line), > > - false) >= > > sizeof(line)); > > + rowsize, sizeof(u32), > > line, > > + sizeof(line)) >= > > sizeof(line)); > > Huh? Why do this? The ascii parameter was removed from the simple API as per Jani's suggestion. The remainder was reformatted to avoid exceeding the line length limits. > > > diff --git a/drivers/isdn/hardware/mISDN/mISDNisar.c > > b/drivers/isdn/hardware/mISDN/mISDNisar.c > [] > > @@ -70,8 +70,9 @@ send_mbox(struct isar_hw *isar, u8 his, u8 creg, > > u8 len, u8 *msg) > > int l = 0; > > > > while (l < (int)len) { > > - hex_dump_to_buffer(msg + l, len - l, > > 32, 1, > > - isar->log, 256, 1); > > + hex_dump_to_buffer_ext(msg + l, len - > > l, 32, 1, > > + isar->log, 256, > > + HEXDUMP_ASCII); > > Again, why do any of these? > > The point of the wrapper is to avoid changing these. Jani made a pretty good point that about half the callers didn't want an ASCII dump, and presenting a simplified API makes sense. I would actually put forward that we consider dropping rowsize from the simplified API too, as most callers use 32, and those that use 16 would probably be OK with 32. Your proposal, on the other hand, only makes sense if there were many callers, and even so, not in the form that you presented, since that result in a mix of booleans & bitfields that you were critical of. -- Alastair D'Silva mob: 0423 762 819 skype: alastair_dsilva Twitter: @EvilDeece blog: http://alastair.d-silva.org ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [driver-core:driver-core-testing 84/85] drivers/s390/cio/device.c:1660:9: warning: assignment discards 'const' qualifier from pointer target type
On Tue, Jun 25, 2019 at 10:28:55PM +0800, kbuild test robot wrote: > tree: > https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/driver-core.git > driver-core-testing > head: 65b66682344a15ba2069d4dd8d0cc39cc3aed7e9 > commit: 92ce7e83b4e5c86687d748ba53cb755acdce1256 [84/85] driver_find_device: > Unify the match function with class_find_device() > config: s390-debug_defconfig (attached as .config) > compiler: s390-linux-gcc (GCC) 7.4.0 > reproduce: > wget > https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O > ~/bin/make.cross > chmod +x ~/bin/make.cross > git checkout 92ce7e83b4e5c86687d748ba53cb755acdce1256 > # save the attached .config to linux build tree > GCC_VERSION=7.4.0 make.cross ARCH=s390 > > If you fix the issue, kindly add following tag > Reported-by: kbuild test robot > > All warnings (new ones prefixed by >>): > >drivers/s390/cio/device.c: In function '__ccwdev_check_busid': > >> drivers/s390/cio/device.c:1660:9: warning: assignment discards 'const' > >> qualifier from pointer target type [-Wdiscarded-qualifiers] > bus_id = id; > ^ > -- >drivers/s390/cio/ccwgroup.c: In function '__ccwgroupdev_check_busid': > >> drivers/s390/cio/ccwgroup.c:613:17: warning: initialization discards > >> 'const' qualifier from pointer target type [-Wdiscarded-qualifiers] > char *bus_id = id; > ^~ Suzuki, can you send me a fix for this please? thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v4 0/7] Hexdump Enhancements
On Mon, 2019-06-24 at 22:01 -0700, Joe Perches wrote: > On Tue, 2019-06-25 at 13:17 +1000, Alastair D'Silva wrote: > > From: Alastair D'Silva > > > > Apologies for the large CC list, it's a heads up for those > > responsible > > for subsystems where a prototype change in generic code causes a > > change > > in those subsystems. > [] > > The default behaviour of hexdump is unchanged, however, the > > prototype > > for hex_dump_to_buffer() has changed, and print_hex_dump() has been > > renamed to print_hex_dump_ext(), with a wrapper replacing it for > > compatibility with existing code, which would have been too > > invasive to > > change. > > I believe this cover letter is misleading. > > The point of the wrapper is to avoid unnecessary changes > in existing > code. > > The wrapper is for print_hex_dump(), which has many callers. The changes to existing code are for hex_dump_to_buffer(), which is called in relatively few places. -- Alastair D'Silva mob: 0423 762 819 skype: alastair_dsilva Twitter: @EvilDeece blog: http://alastair.d-silva.org ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v4 4/7] lib/hexdump.c: Replace ascii bool in hex_dump_to_buffer with flags
Hi Alastair, Thank you for the patch! Perhaps something to improve: [auto build test WARNING on linus/master] [also build test WARNING on v5.2-rc6 next-20190625] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Alastair-D-Silva/Hexdump-Enhancements/20190625-224046 reproduce: # apt-get install sparse # sparse version: v0.6.1-rc1-7-g2b96cd8-dirty make ARCH=x86_64 allmodconfig make C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' If you fix the issue, kindly add following tag Reported-by: kbuild test robot sparse warnings: (new ones prefixed by >>) sound/soc/intel/skylake/skl-debug.c:191:34: sparse: sparse: incorrect type in argument 1 (different address spaces) @@expected void [noderef] *to @@got eref] *to @@ sound/soc/intel/skylake/skl-debug.c:191:34: sparse:expected void [noderef] *to sound/soc/intel/skylake/skl-debug.c:191:34: sparse:got unsigned char * sound/soc/intel/skylake/skl-debug.c:191:51: sparse: sparse: incorrect type in argument 2 (different address spaces) @@expected void const *from @@ got void [noderef] void const *from @@ sound/soc/intel/skylake/skl-debug.c:191:51: sparse:expected void const *from sound/soc/intel/skylake/skl-debug.c:191:51: sparse:got void [noderef] *[assigned] fw_reg_addr >> sound/soc/intel/skylake/skl-debug.c:195:35: sparse: sparse: too many >> arguments for function hex_dump_to_buffer -- >> drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c:93:27: sparse: sparse: too >> many arguments for function hex_dump_to_buffer -- >> sound/soc/sof/xtensa/core.c:125:35: sparse: sparse: too many arguments for >> function hex_dump_to_buffer vim +195 sound/soc/intel/skylake/skl-debug.c d14700a0 Vinod Koul 2017-06-30 170 bdd0384a Vunny Sodhi 2017-06-30 171 static ssize_t fw_softreg_read(struct file *file, char __user *user_buf, bdd0384a Vunny Sodhi 2017-06-30 172 size_t count, loff_t *ppos) bdd0384a Vunny Sodhi 2017-06-30 173 { bdd0384a Vunny Sodhi 2017-06-30 174struct skl_debug *d = file->private_data; bdd0384a Vunny Sodhi 2017-06-30 175struct sst_dsp *sst = d->skl->skl_sst->dsp; bdd0384a Vunny Sodhi 2017-06-30 176size_t w0_stat_sz = sst->addr.w0_stat_sz; bdd0384a Vunny Sodhi 2017-06-30 177void __iomem *in_base = sst->mailbox.in_base; bdd0384a Vunny Sodhi 2017-06-30 178void __iomem *fw_reg_addr; bdd0384a Vunny Sodhi 2017-06-30 179unsigned int offset; bdd0384a Vunny Sodhi 2017-06-30 180char *tmp; bdd0384a Vunny Sodhi 2017-06-30 181ssize_t ret = 0; bdd0384a Vunny Sodhi 2017-06-30 182 bdd0384a Vunny Sodhi 2017-06-30 183tmp = kzalloc(FW_REG_BUF, GFP_KERNEL); bdd0384a Vunny Sodhi 2017-06-30 184if (!tmp) bdd0384a Vunny Sodhi 2017-06-30 185return -ENOMEM; bdd0384a Vunny Sodhi 2017-06-30 186 bdd0384a Vunny Sodhi 2017-06-30 187fw_reg_addr = in_base - w0_stat_sz; bdd0384a Vunny Sodhi 2017-06-30 188memset(d->fw_read_buff, 0, FW_REG_BUF); bdd0384a Vunny Sodhi 2017-06-30 189 bdd0384a Vunny Sodhi 2017-06-30 190if (w0_stat_sz > 0) bdd0384a Vunny Sodhi 2017-06-30 @191 __iowrite32_copy(d->fw_read_buff, fw_reg_addr, w0_stat_sz >> 2); bdd0384a Vunny Sodhi 2017-06-30 192 bdd0384a Vunny Sodhi 2017-06-30 193for (offset = 0; offset < FW_REG_SIZE; offset += 16) { bdd0384a Vunny Sodhi 2017-06-30 194ret += snprintf(tmp + ret, FW_REG_BUF - ret, "%#.4x: ", offset); bdd0384a Vunny Sodhi 2017-06-30 @195 hex_dump_to_buffer(d->fw_read_buff + offset, 16, 16, 4, bdd0384a Vunny Sodhi 2017-06-30 196 tmp + ret, FW_REG_BUF - ret, 0); bdd0384a Vunny Sodhi 2017-06-30 197ret += strlen(tmp + ret); bdd0384a Vunny Sodhi 2017-06-30 198 bdd0384a Vunny Sodhi 2017-06-30 199/* print newline for each offset */ bdd0384a Vunny Sodhi 2017-06-30 200if (FW_REG_BUF - ret > 0) bdd0384a Vunny Sodhi 2017-06-30 201tmp[ret++] = '\n'; bdd0384a Vunny Sodhi 2017-06-30 202} bdd0384a Vunny Sodhi 2017-06-30 203 bdd0384a Vunny Sodhi 2017-06-30 204ret = simple_read_from_buffer(user_buf, count, ppos, tmp, ret); bdd0384a Vunny Sodhi 2017-06-30 205kfree(tmp); bdd0384a Vunny Sodhi 2017-06-30 206 bdd0384a Vunny Sodhi 2017-06-30 207return ret; bdd0384a Vunny Sodhi 2017-06-30 208 } bdd0384a Vunny Sodhi 2017-06-30 209 :: The code at line 195 was first introduced by commit :: bdd0384a5ada8bb5745e5f29c10a5ba88827efad ASoC: Intel: Skylake: Add support to read firmware registers :: TO: Vunny Sodhi :: CC: Mark Brown --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all
Re: [PATCH v4 3/7] lib/hexdump.c: Optionally suppress lines of repeated bytes
Hi Alastair, Thank you for the patch! Perhaps something to improve: [auto build test WARNING on linus/master] [also build test WARNING on v5.2-rc6 next-20190625] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Alastair-D-Silva/Hexdump-Enhancements/20190625-224046 config: xtensa-allyesconfig (attached as .config) compiler: xtensa-linux-gcc (GCC) 7.4.0 reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree GCC_VERSION=7.4.0 make.cross ARCH=xtensa If you fix the issue, kindly add following tag Reported-by: kbuild test robot All warnings (new ones prefixed by >>): lib/hexdump.c: In function 'announce_skipped': >> lib/hexdump.c:243:28: warning: format '%lu' expects argument of type 'long >> unsigned int', but argument 4 has type 'size_t {aka unsigned int}' >> [-Wformat=] printk("%s%s ** Skipped %lu bytes of value 0x%x **\n", ~~^ %u vim +243 lib/hexdump.c 236 237 static void announce_skipped(const char *level, const char *prefix_str, 238 u8 val, size_t count) 239 { 240 if (count == 0) 241 return; 242 > 243 printk("%s%s ** Skipped %lu bytes of value 0x%x **\n", 244 level, prefix_str, count, val); 245 } 246 --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: application/gzip ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 7/9] staging: vc04_services: Remove vchiq_send_remote_release()
Remove unused function vchiq_send_remote_release. Issue found with Coccinelle. Signed-off-by: Nishka Dasgupta --- .../vc04_services/interface/vchiq_arm/vchiq_core.c| 11 --- .../vc04_services/interface/vchiq_arm/vchiq_core.h| 3 --- 2 files changed, 14 deletions(-) diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c index 5e231cc5c87d..183f5cf887e0 100644 --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c @@ -3538,17 +3538,6 @@ VCHIQ_STATUS_T vchiq_send_remote_use(struct vchiq_state *state) return status; } -VCHIQ_STATUS_T vchiq_send_remote_release(struct vchiq_state *state) -{ - VCHIQ_STATUS_T status = VCHIQ_RETRY; - - if (state->conn_state != VCHIQ_CONNSTATE_DISCONNECTED) - status = queue_message(state, NULL, - VCHIQ_MAKE_MSG(VCHIQ_MSG_REMOTE_RELEASE, 0, 0), - NULL, NULL, 0, 0); - return status; -} - VCHIQ_STATUS_T vchiq_send_remote_use_active(struct vchiq_state *state) { VCHIQ_STATUS_T status = VCHIQ_RETRY; diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h index b5e09d52b202..63f71b2a492f 100644 --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h @@ -639,9 +639,6 @@ vchiq_on_remote_use_active(struct vchiq_state *state); extern VCHIQ_STATUS_T vchiq_send_remote_use(struct vchiq_state *state); -extern VCHIQ_STATUS_T -vchiq_send_remote_release(struct vchiq_state *state); - extern VCHIQ_STATUS_T vchiq_send_remote_use_active(struct vchiq_state *state); -- 2.19.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/9] staging: vc04_services: Remove function vchiq_arm_allow_resume()
Remove unused function vchiq_arm_allow_resume. Issue found with Coccinelle. Signed-off-by: Nishka Dasgupta --- .../interface/vchiq_arm/vchiq_arm.c | 43 --- .../interface/vchiq_arm/vchiq_arm.h | 3 -- 2 files changed, 46 deletions(-) diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c index 9264a07cf160..bf7c1e2bce67 100644 --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c @@ -2883,49 +2883,6 @@ vchiq_check_suspend(struct vchiq_state *state) vchiq_log_trace(vchiq_susp_log_level, "%s exit", __func__); } -int -vchiq_arm_allow_resume(struct vchiq_state *state) -{ - struct vchiq_arm_state *arm_state = vchiq_platform_get_arm_state(state); - int resume = 0; - int ret = -1; - - if (!arm_state) - goto out; - - vchiq_log_trace(vchiq_susp_log_level, "%s", __func__); - - write_lock_bh(_state->susp_res_lock); - unblock_resume(arm_state); - resume = vchiq_check_resume(state); - write_unlock_bh(_state->susp_res_lock); - - if (resume) { - if (wait_for_completion_interruptible( - _state->vc_resume_complete) < 0) { - vchiq_log_error(vchiq_susp_log_level, - "%s interrupted", __func__); - /* failed, cannot accurately derive suspend -* state, so exit early. */ - goto out; - } - } - - read_lock_bh(_state->susp_res_lock); - if (arm_state->vc_suspend_state == VC_SUSPEND_SUSPENDED) { - vchiq_log_info(vchiq_susp_log_level, - "%s: Videocore remains suspended", __func__); - } else { - vchiq_log_info(vchiq_susp_log_level, - "%s: Videocore resumed", __func__); - ret = 0; - } - read_unlock_bh(_state->susp_res_lock); -out: - vchiq_log_trace(vchiq_susp_log_level, "%s exit %d", __func__, ret); - return ret; -} - /* This function should be called with the write lock held */ int vchiq_check_resume(struct vchiq_state *state) diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.h b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.h index c1d5a9d17071..61b15278c999 100644 --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.h +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.h @@ -115,9 +115,6 @@ vchiq_arm_vcsuspend(struct vchiq_state *state); extern VCHIQ_STATUS_T vchiq_arm_force_suspend(struct vchiq_state *state); -extern int -vchiq_arm_allow_resume(struct vchiq_state *state); - extern VCHIQ_STATUS_T vchiq_arm_vcresume(struct vchiq_state *state); -- 2.19.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 6/9] staging: vc04_services: Remove vchiq_use_service_no_resume()
Remove unused function vchiq_use_service_no_resume. Issue found with Coccinelle. Signed-off-by: Nishka Dasgupta --- .../vc04_services/interface/vchiq_arm/vchiq_arm.c | 14 -- .../vc04_services/interface/vchiq_arm/vchiq_if.h | 2 -- 2 files changed, 16 deletions(-) diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c index 6e59470d44ab..a97076c18a0f 100644 --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c @@ -3072,20 +3072,6 @@ static void suspend_timer_callback(struct timer_list *t) vchiq_check_suspend(state); } -VCHIQ_STATUS_T -vchiq_use_service_no_resume(VCHIQ_SERVICE_HANDLE_T handle) -{ - VCHIQ_STATUS_T ret = VCHIQ_ERROR; - struct vchiq_service *service = find_service_by_handle(handle); - - if (service) { - ret = vchiq_use_internal(service->state, service, - USE_TYPE_SERVICE_NO_RESUME); - unlock_service(service); - } - return ret; -} - VCHIQ_STATUS_T vchiq_use_service(VCHIQ_SERVICE_HANDLE_T handle) { diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_if.h b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_if.h index 5445f201e284..c23bd105c40f 100644 --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_if.h +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_if.h @@ -107,8 +107,6 @@ extern VCHIQ_STATUS_T vchiq_open_service(VCHIQ_INSTANCE_T instance, extern VCHIQ_STATUS_T vchiq_close_service(VCHIQ_SERVICE_HANDLE_T service); extern VCHIQ_STATUS_T vchiq_remove_service(VCHIQ_SERVICE_HANDLE_T service); extern VCHIQ_STATUS_T vchiq_use_service(VCHIQ_SERVICE_HANDLE_T service); -extern VCHIQ_STATUS_T vchiq_use_service_no_resume( - VCHIQ_SERVICE_HANDLE_T service); extern VCHIQ_STATUS_T vchiq_release_service(VCHIQ_SERVICE_HANDLE_T service); extern VCHIQ_STATUS_T vchiq_queue_message(VCHIQ_SERVICE_HANDLE_T handle, -- 2.19.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 9/9] staging: vc04_services: Remove function block_resume()
Remove function block_resume as it was only called by vchiq_arm_force_suspend, which was removed in a previous patch. Issue found with Coccinelle. Signed-off-by: Nishka Dasgupta --- .../interface/vchiq_arm/vchiq_arm.c | 66 --- 1 file changed, 66 deletions(-) diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c index ebf7e2a3bd3b..cc4383d1ec3e 100644 --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c @@ -2554,72 +2554,6 @@ need_resume(struct vchiq_state *state) vchiq_videocore_wanted(state); } -static int -block_resume(struct vchiq_arm_state *arm_state) -{ - int status = VCHIQ_SUCCESS; - const unsigned long timeout_val = - msecs_to_jiffies(FORCE_SUSPEND_TIMEOUT_MS); - int resume_count = 0; - - /* Allow any threads which were blocked by the last force suspend to -* complete if they haven't already. Only give this one shot; if -* blocked_count is incremented after blocked_blocker is completed -* (which only happens when blocked_count hits 0) then those threads -* will have to wait until next time around */ - if (arm_state->blocked_count) { - reinit_completion(_state->blocked_blocker); - write_unlock_bh(_state->susp_res_lock); - vchiq_log_info(vchiq_susp_log_level, "%s wait for previously " - "blocked clients", __func__); - if (wait_for_completion_interruptible_timeout( - _state->blocked_blocker, timeout_val) - <= 0) { - vchiq_log_error(vchiq_susp_log_level, "%s wait for " - "previously blocked clients failed", __func__); - status = VCHIQ_ERROR; - write_lock_bh(_state->susp_res_lock); - goto out; - } - vchiq_log_info(vchiq_susp_log_level, "%s previously blocked " - "clients resumed", __func__); - write_lock_bh(_state->susp_res_lock); - } - - /* We need to wait for resume to complete if it's in process */ - while (arm_state->vc_resume_state != VC_RESUME_RESUMED && - arm_state->vc_resume_state > VC_RESUME_IDLE) { - if (resume_count > 1) { - status = VCHIQ_ERROR; - vchiq_log_error(vchiq_susp_log_level, "%s waited too " - "many times for resume", __func__); - goto out; - } - write_unlock_bh(_state->susp_res_lock); - vchiq_log_info(vchiq_susp_log_level, "%s wait for resume", - __func__); - if (wait_for_completion_interruptible_timeout( - _state->vc_resume_complete, timeout_val) - <= 0) { - vchiq_log_error(vchiq_susp_log_level, "%s wait for " - "resume failed (%s)", __func__, - resume_state_names[arm_state->vc_resume_state + - VC_RESUME_NUM_OFFSET]); - status = VCHIQ_ERROR; - write_lock_bh(_state->susp_res_lock); - goto out; - } - vchiq_log_info(vchiq_susp_log_level, "%s resumed", __func__); - write_lock_bh(_state->susp_res_lock); - resume_count++; - } - reinit_completion(_state->resume_blocker); - arm_state->resume_blocked = 1; - -out: - return status; -} - static inline void unblock_resume(struct vchiq_arm_state *arm_state) { -- 2.19.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 5/9] staging: vc04_services: Remove vchiq_resume_internal()
Remove unused function vchiq_resume_internal. Issue found with Coccinelle. Signed-off-by: Nishka Dasgupta --- .../interface/vchiq_arm/vchiq_core.c | 16 .../interface/vchiq_arm/vchiq_core.h | 3 --- 2 files changed, 19 deletions(-) diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c index 7f093b2679ae..5e231cc5c87d 100644 --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c @@ -2830,22 +2830,6 @@ vchiq_shutdown_internal(struct vchiq_state *state, VCHIQ_INSTANCE_T instance) return VCHIQ_SUCCESS; } -VCHIQ_STATUS_T -vchiq_resume_internal(struct vchiq_state *state) -{ - VCHIQ_STATUS_T status = VCHIQ_SUCCESS; - - if (state->conn_state == VCHIQ_CONNSTATE_PAUSED) { - vchiq_set_conn_state(state, VCHIQ_CONNSTATE_RESUMING); - request_poll(state, NULL, 0); - } else { - status = VCHIQ_ERROR; - VCHIQ_STATS_INC(state, error_count); - } - - return status; -} - VCHIQ_STATUS_T vchiq_close_service(VCHIQ_SERVICE_HANDLE_T handle) { diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h index b319031145ed..b5e09d52b202 100644 --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h @@ -518,9 +518,6 @@ vchiq_free_service_internal(struct vchiq_service *service); extern VCHIQ_STATUS_T vchiq_shutdown_internal(struct vchiq_state *state, VCHIQ_INSTANCE_T instance); -extern VCHIQ_STATUS_T -vchiq_resume_internal(struct vchiq_state *state); - extern void remote_event_pollall(struct vchiq_state *state); -- 2.19.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 4/9] staging: vc04_services: Remove vchiq_pause_internal()
Remove unused function vchiq_pause_internal. Issue found with Coccinelle. Signed-off-by: Nishka Dasgupta --- .../interface/vchiq_arm/vchiq_core.c | 23 --- .../interface/vchiq_arm/vchiq_core.h | 3 --- 2 files changed, 26 deletions(-) diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c index 0dca6e834ffa..7f093b2679ae 100644 --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c @@ -2830,29 +2830,6 @@ vchiq_shutdown_internal(struct vchiq_state *state, VCHIQ_INSTANCE_T instance) return VCHIQ_SUCCESS; } -VCHIQ_STATUS_T -vchiq_pause_internal(struct vchiq_state *state) -{ - VCHIQ_STATUS_T status = VCHIQ_SUCCESS; - - switch (state->conn_state) { - case VCHIQ_CONNSTATE_CONNECTED: - /* Request a pause */ - vchiq_set_conn_state(state, VCHIQ_CONNSTATE_PAUSING); - request_poll(state, NULL, 0); - break; - default: - vchiq_log_error(vchiq_core_log_level, - "%s in state %s\n", - __func__, conn_state_names[state->conn_state]); - status = VCHIQ_ERROR; - VCHIQ_STATS_INC(state, error_count); - break; - } - - return status; -} - VCHIQ_STATUS_T vchiq_resume_internal(struct vchiq_state *state) { diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h index aee2d362e88d..b319031145ed 100644 --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h @@ -518,9 +518,6 @@ vchiq_free_service_internal(struct vchiq_service *service); extern VCHIQ_STATUS_T vchiq_shutdown_internal(struct vchiq_state *state, VCHIQ_INSTANCE_T instance); -extern VCHIQ_STATUS_T -vchiq_pause_internal(struct vchiq_state *state); - extern VCHIQ_STATUS_T vchiq_resume_internal(struct vchiq_state *state); -- 2.19.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 8/9] staging: vc04_services: Remove function output_timeout_error()
Remove function output_timeout_error as it was only called by vchiq_arm_force_suspend, which was deleted in a previous patch. Issue found with Coccinelle. Signed-off-by: Nishka Dasgupta --- .../interface/vchiq_arm/vchiq_arm.c | 36 --- 1 file changed, 36 deletions(-) diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c index a97076c18a0f..ebf7e2a3bd3b 100644 --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c @@ -2705,42 +2705,6 @@ vchiq_platform_check_suspend(struct vchiq_state *state) return; } -static void -output_timeout_error(struct vchiq_state *state) -{ - struct vchiq_arm_state *arm_state = vchiq_platform_get_arm_state(state); - char err[50] = ""; - int vc_use_count = arm_state->videocore_use_count; - int active_services = state->unused_service; - int i; - - if (!arm_state->videocore_use_count) { - snprintf(err, sizeof(err), " Videocore usecount is 0"); - goto output_msg; - } - for (i = 0; i < active_services; i++) { - struct vchiq_service *service_ptr = state->services[i]; - - if (service_ptr && service_ptr->service_use_count && - (service_ptr->srvstate != VCHIQ_SRVSTATE_FREE)) { - snprintf(err, sizeof(err), " %c%c%c%c(%d) service has " - "use count %d%s", VCHIQ_FOURCC_AS_4CHARS( - service_ptr->base.fourcc), -service_ptr->client_id, -service_ptr->service_use_count, -service_ptr->service_use_count == -vc_use_count ? "" : " (+ more)"); - break; - } - } - -output_msg: - vchiq_log_error(vchiq_susp_log_level, - "timed out waiting for vc suspend (%d).%s", -arm_state->autosuspend_override, err); - -} - void vchiq_check_suspend(struct vchiq_state *state) { -- 2.19.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 3/9] staging: vc04_services: Remove vchiq_arm_force_suspend()
Remove unused function vchiq_arm_force_suspend. Issue found with Coccinelle. Signed-off-by: Nishka Dasgupta --- .../interface/vchiq_arm/vchiq_arm.c | 120 -- .../interface/vchiq_arm/vchiq_arm.h | 3 - 2 files changed, 123 deletions(-) diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c index bf7c1e2bce67..6e59470d44ab 100644 --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c @@ -2741,126 +2741,6 @@ output_timeout_error(struct vchiq_state *state) } -/* Try to get videocore into suspended state, regardless of autosuspend state. -** We don't actually force suspend, since videocore may get into a bad state -** if we force suspend at a bad time. Instead, we wait for autosuspend to -** determine a good point to suspend. If this doesn't happen within 100ms we -** report failure. -** -** Returns VCHIQ_SUCCESS if videocore suspended successfully, VCHIQ_RETRY if -** videocore failed to suspend in time or VCHIQ_ERROR if interrupted. -*/ -VCHIQ_STATUS_T -vchiq_arm_force_suspend(struct vchiq_state *state) -{ - struct vchiq_arm_state *arm_state = vchiq_platform_get_arm_state(state); - VCHIQ_STATUS_T status = VCHIQ_ERROR; - long rc = 0; - int repeat = -1; - - if (!arm_state) - goto out; - - vchiq_log_trace(vchiq_susp_log_level, "%s", __func__); - - write_lock_bh(_state->susp_res_lock); - - status = block_resume(arm_state); - if (status != VCHIQ_SUCCESS) - goto unlock; - if (arm_state->vc_suspend_state == VC_SUSPEND_SUSPENDED) { - /* Already suspended - just block resume and exit */ - vchiq_log_info(vchiq_susp_log_level, "%s already suspended", - __func__); - status = VCHIQ_SUCCESS; - goto unlock; - } else if (arm_state->vc_suspend_state <= VC_SUSPEND_IDLE) { - /* initiate suspend immediately in the case that we're waiting -* for the timeout */ - stop_suspend_timer(arm_state); - if (!vchiq_videocore_wanted(state)) { - vchiq_log_info(vchiq_susp_log_level, "%s videocore " - "idle, initiating suspend", __func__); - status = vchiq_arm_vcsuspend(state); - } else if (arm_state->autosuspend_override < - FORCE_SUSPEND_FAIL_MAX) { - vchiq_log_info(vchiq_susp_log_level, "%s letting " - "videocore go idle", __func__); - status = VCHIQ_SUCCESS; - } else { - vchiq_log_warning(vchiq_susp_log_level, "%s failed too " - "many times - attempting suspend", __func__); - status = vchiq_arm_vcsuspend(state); - } - } else { - vchiq_log_info(vchiq_susp_log_level, "%s videocore suspend " - "in progress - wait for completion", __func__); - status = VCHIQ_SUCCESS; - } - - /* Wait for suspend to happen due to system idle (not forced..) */ - if (status != VCHIQ_SUCCESS) - goto unblock_resume; - - do { - write_unlock_bh(_state->susp_res_lock); - - rc = wait_for_completion_interruptible_timeout( - _state->vc_suspend_complete, - msecs_to_jiffies(FORCE_SUSPEND_TIMEOUT_MS)); - - write_lock_bh(_state->susp_res_lock); - if (rc < 0) { - vchiq_log_warning(vchiq_susp_log_level, "%s " - "interrupted waiting for suspend", __func__); - status = VCHIQ_ERROR; - goto unblock_resume; - } else if (rc == 0) { - if (arm_state->vc_suspend_state > VC_SUSPEND_IDLE) { - /* Repeat timeout once if in progress */ - if (repeat < 0) { - repeat = 1; - continue; - } - } - arm_state->autosuspend_override++; - output_timeout_error(state); - - status = VCHIQ_RETRY; - goto unblock_resume; - } - } while (0 < (repeat--)); - - /* Check and report state in case we need to abort ARM suspend */ - if (arm_state->vc_suspend_state != VC_SUSPEND_SUSPENDED) { - status = VCHIQ_RETRY; - vchiq_log_error(vchiq_susp_log_level, - "%s videocore suspend failed (state
[PATCH 1/9] staging: vc04_services: Remove function vchiu_queue_is_full()
Remove unused function vchiu_queue_is_full. Issue found with Coccinelle. Signed-off-by: Nishka Dasgupta --- .../staging/vc04_services/interface/vchiq_arm/vchiq_util.c | 5 - .../staging/vc04_services/interface/vchiq_arm/vchiq_util.h | 1 - 2 files changed, 6 deletions(-) diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_util.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_util.c index 8ee85c5e6f77..5e6d3035dc05 100644 --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_util.c +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_util.c @@ -39,11 +39,6 @@ int vchiu_queue_is_empty(struct vchiu_queue *queue) return queue->read == queue->write; } -int vchiu_queue_is_full(struct vchiu_queue *queue) -{ - return queue->write == queue->read + queue->size; -} - void vchiu_queue_push(struct vchiu_queue *queue, struct vchiq_header *header) { if (!queue->initialized) diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_util.h b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_util.h index ee1459468171..f03a4250de0d 100644 --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_util.h +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_util.h @@ -40,7 +40,6 @@ extern int vchiu_queue_init(struct vchiu_queue *queue, int size); extern void vchiu_queue_delete(struct vchiu_queue *queue); extern int vchiu_queue_is_empty(struct vchiu_queue *queue); -extern int vchiu_queue_is_full(struct vchiu_queue *queue); extern void vchiu_queue_push(struct vchiu_queue *queue, struct vchiq_header *header); -- 2.19.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v4 4/7] lib/hexdump.c: Replace ascii bool in hex_dump_to_buffer with flags
Hi Alastair, Thank you for the patch! Yet something to improve: [auto build test ERROR on linus/master] [also build test ERROR on v5.2-rc6 next-20190625] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Alastair-D-Silva/Hexdump-Enhancements/20190625-224046 config: sh-allmodconfig (attached as .config) compiler: sh4-linux-gcc (GCC) 7.4.0 reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree GCC_VERSION=7.4.0 make.cross ARCH=sh If you fix the issue, kindly add following tag Reported-by: kbuild test robot All errors (new ones prefixed by >>): drivers/gpu//drm/tinydrm/core/tinydrm-helpers.c: In function 'tinydrm_dbg_spi_print': >> drivers/gpu//drm/tinydrm/core/tinydrm-helpers.c:93:2: error: too many >> arguments to function 'hex_dump_to_buffer' hex_dump_to_buffer(buf, tr->len, 16, ^~ In file included from include/linux/kernel.h:15:0, from include/linux/list.h:9, from include/linux/kobject.h:19, from include/linux/device.h:16, from include/linux/backlight.h:12, from drivers/gpu//drm/tinydrm/core/tinydrm-helpers.c:6: include/linux/printk.h:523:19: note: declared here static inline int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, ^~ -- sound/soc//sof/xtensa/core.c: In function 'xtensa_stack': >> sound/soc//sof/xtensa/core.c:125:3: error: too many arguments to function >> 'hex_dump_to_buffer' hex_dump_to_buffer(stack + i * 4, 16, 16, 4, ^~ In file included from include/linux/kernel.h:15:0, from include/linux/list.h:9, from include/linux/module.h:9, from sound/soc//sof/xtensa/core.c:11: include/linux/printk.h:523:19: note: declared here static inline int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, ^~ vim +/hex_dump_to_buffer +93 drivers/gpu//drm/tinydrm/core/tinydrm-helpers.c 9f69eb5c Noralf Trønnes 2017-01-22 85 9f69eb5c Noralf Trønnes 2017-01-22 86 static void 9f69eb5c Noralf Trønnes 2017-01-22 87 tinydrm_dbg_spi_print(struct spi_device *spi, struct spi_transfer *tr, 9f69eb5c Noralf Trønnes 2017-01-22 88 const void *buf, int idx, bool tx) 9f69eb5c Noralf Trønnes 2017-01-22 89 { 9f69eb5c Noralf Trønnes 2017-01-22 90 u32 speed_hz = tr->speed_hz ? tr->speed_hz : spi->max_speed_hz; 9f69eb5c Noralf Trønnes 2017-01-22 91 char linebuf[3 * 32]; 9f69eb5c Noralf Trønnes 2017-01-22 92 9f69eb5c Noralf Trønnes 2017-01-22 @93 hex_dump_to_buffer(buf, tr->len, 16, 9f69eb5c Noralf Trønnes 2017-01-22 94 DIV_ROUND_UP(tr->bits_per_word, 8), 9f69eb5c Noralf Trønnes 2017-01-22 95linebuf, sizeof(linebuf), false); 9f69eb5c Noralf Trønnes 2017-01-22 96 9f69eb5c Noralf Trønnes 2017-01-22 97 printk(KERN_DEBUG 9f69eb5c Noralf Trønnes 2017-01-22 98"tr(%i): speed=%u%s, bpw=%i, len=%u, %s_buf=[%s%s]\n", idx, 9f69eb5c Noralf Trønnes 2017-01-22 99speed_hz > 100 ? speed_hz / 100 : speed_hz / 1000, 9f69eb5c Noralf Trønnes 2017-01-22 100speed_hz > 100 ? "MHz" : "kHz", tr->bits_per_word, tr->len, 9f69eb5c Noralf Trønnes 2017-01-22 101tx ? "tx" : "rx", linebuf, tr->len > 16 ? " ..." : ""); 9f69eb5c Noralf Trønnes 2017-01-22 102 } 9f69eb5c Noralf Trønnes 2017-01-22 103 :: The code at line 93 was first introduced by commit :: 9f69eb5c36a644571cca6b2f8dc5f6a7cba04a8b drm/tinydrm: Add helper functions :: TO: Noralf Trønnes :: CC: Noralf Trønnes --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: application/gzip ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: bcm2835-camera: Restore return behavior of ctrl_set_bitrate()
Hi Dan, hi Dave, Am 25.06.19 um 09:55 schrieb Dan Carpenter: > On Tue, Jun 25, 2019 at 12:13:15AM +0200, Stefan Wahren wrote: >> The commit 52c4dfcead49 ("Staging: vc04_services: Cleanup in >> ctrl_set_bitrate()") changed the return behavior of ctrl_set_bitrate(). >> This breaks probing of bcm2835-camera: >> >> bcm2835-v4l2: mmal_init: failed to set all camera controls: -3 >> Cleanup: Destroy video encoder >> Cleanup: Destroy image encoder >> Cleanup: Destroy video render >> Cleanup: Destroy camera >> bcm2835-v4l2: bcm2835_mmal_probe: mmal init failed: -3 >> bcm2835-camera: probe of bcm2835-camera failed with error -3 >> >> So restore the old behavior and fix this issue. >> >> Fixes: 52c4dfcead49 ("Staging: vc04_services: Cleanup in ctrl_set_bitrate()") >> Signed-off-by: Stefan Wahren > I feel like this papers over the issue. It would be better to figure > out why this is failing and fix it properly. -3 is -ESRCH and when I > grep for ESRCH I only see it used in the ioctl so that can't be it. > > I think it must be -MMAL_MSG_STATUS_EINVAL actually, but it comes from > the firmware or something so we can't grep for it. yes this is a result from the VC4 firmware, there is nothing i can do about it. Even the firmware has been fixed, the driver must be compatible with older firmware version. > Can we do some more digging to find out why it's failing or otherwise > we could add a comment. > > /* >* FIXME: port_parameter_set() sometimes fails with >* -MMAL_MSG_STATUS_EINVAL and we don't know why so we're >* ignoring those errors for now. >* >*/ > return 0; I will add a comment and a v4l2_dbg entry. @Dave I used a Raspberry Pi 3 with a V1.3 camera and get this with an additional v4l2_dbg in ctrl_set_bitrate() [ 1.472805] raspberrypi-firmware soc:firmware: Attached to firmware from 2019-03-27 15:48 ... [ 7.441639] videodev: Linux video capture interface: v2.00 [ 7.511659] bcm2835_v4l2: module is from the staging directory, the quality is unknown, you have been warned. ... [ 8.162504] bcm2835-v4l2: Set fps range to 3/1000 to 3/1000 [ 8.163104] bcm2835-v4l2: Set fps range to 3/1000 to 3/1000 [ 8.163624] bcm2835-v4l2: Set fps range to 3/1000 to 3/1000 [ 8.164395] bcm2835-v4l2: mmal_ctrl:eecd7187 ctrl id:0x98091f ctrl val:0 imagefx:0x0 color_effect:false u:0 v:0 ret 0(0) [ 8.164493] bcm2835-v4l2: ctrl_set_colfx: After: mmal_ctrl:1ec18e37 ctrl id:0x98092a ctrl val:32896 ret 0(0) [ 8.165413] bcm2835-v4l2: ctrl_set_bitrate: After: mmal_ctrl:b01a3b48 ctrl id:0x9909cf ctrl val:1000 ret -3(-22) [ 8.165872] bcm2835-v4l2: scene mode selected 0, was 0 [ 8.166249] bcm2835-v4l2: V4L2 device registered as video0 - stills mode > 1280x720 [ 8.166596] bcm2835-v4l2: vid_cap - set up encode comp [ 8.171208] bcm2835-v4l2: JPG - buf size now 786432 was 786432 [ 8.171220] bcm2835-v4l2: vid_cap - cur_buf.size set to 786432 [ 8.171228] bcm2835-v4l2: Set dev->capture.fmt 4745504A, 1024x768, stride 0, size 786432 [ 8.171234] bcm2835-v4l2: Broadcom 2835 MMAL video capture ver 0.0.2 loaded. Looks like the firmware dislike val:1000 Any thoughts? > > > regards, > dan carpenter > > > ___ > linux-arm-kernel mailing list > linux-arm-ker...@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] media: ipu3-imgu: Fixed some coding style issues in ipu3-css.c
Improved readability by fixing some issues related to maximum line length. Signed-off-by: Felix Winkler Signed-off-by: Niklas Witzel --- drivers/staging/media/ipu3/ipu3-css.c | 274 +++--- 1 file changed, 121 insertions(+), 153 deletions(-) diff --git a/drivers/staging/media/ipu3/ipu3-css.c b/drivers/staging/media/ipu3/ipu3-css.c index 23cf5b2..a34105f 100644 --- a/drivers/staging/media/ipu3/ipu3-css.c +++ b/drivers/staging/media/ipu3/ipu3-css.c @@ -663,17 +663,16 @@ static void imgu_css_hw_cleanup(struct imgu_css *css) static void imgu_css_pipeline_cleanup(struct imgu_css *css, unsigned int pipe) { struct imgu_device *imgu = dev_get_drvdata(css->dev); + struct imgu_css_pipe *css_pipe = >pipes[pipe]; unsigned int i; - imgu_css_pool_cleanup(imgu, - >pipes[pipe].pool.parameter_set_info); - imgu_css_pool_cleanup(imgu, >pipes[pipe].pool.acc); - imgu_css_pool_cleanup(imgu, >pipes[pipe].pool.gdc); - imgu_css_pool_cleanup(imgu, >pipes[pipe].pool.obgrid); + imgu_css_pool_cleanup(imgu, _pipe->pool.parameter_set_info); + imgu_css_pool_cleanup(imgu, _pipe->pool.acc); + imgu_css_pool_cleanup(imgu, _pipe->pool.gdc); + imgu_css_pool_cleanup(imgu, _pipe->pool.obgrid); for (i = 0; i < IMGU_ABI_NUM_MEMORIES; i++) - imgu_css_pool_cleanup(imgu, - >pipes[pipe].pool.binary_params_p[i]); + imgu_css_pool_cleanup(imgu, _pipe->pool.binary_params_p[i]); } /* @@ -699,6 +698,12 @@ static int imgu_css_pipeline_init(struct imgu_css *css, unsigned int pipe) unsigned int i, j; struct imgu_css_pipe *css_pipe = >pipes[pipe]; + struct imgu_css_queue *css_queue_in = + _pipe->queue[IPU3_CSS_QUEUE_IN]; + struct imgu_css_queue *css_queue_out = + _pipe->queue[IPU3_CSS_QUEUE_OUT]; + struct imgu_css_queue *css_queue_vf = + _pipe->queue[IPU3_CSS_QUEUE_VF]; const struct imgu_fw_info *bi = >fwp->binary_header[css_pipe->bindex]; const unsigned int stripes = bi->info.isp.sp.iterator.num_stripes; @@ -711,6 +716,9 @@ static int imgu_css_pipeline_init(struct imgu_css *css, unsigned int pipe) struct imgu_abi_isp_stage *isp_stage; struct imgu_abi_sp_stage *sp_stage; struct imgu_abi_sp_group *sp_group; + struct imgu_abi_frames_sp *frames_sp; + struct imgu_abi_frame_sp *frame_sp; + struct imgu_abi_frame_sp_info *frame_sp_info; const unsigned int bds_width_pad = ALIGN(css_pipe->rect[IPU3_CSS_RECT_BDS].width, @@ -732,61 +740,44 @@ static int imgu_css_pipeline_init(struct imgu_css *css, unsigned int pipe) if (!cfg_iter) goto bad_firmware; - cfg_iter->input_info.res.width = - css_pipe->queue[IPU3_CSS_QUEUE_IN].fmt.mpix.width; - cfg_iter->input_info.res.height = - css_pipe->queue[IPU3_CSS_QUEUE_IN].fmt.mpix.height; - cfg_iter->input_info.padded_width = - css_pipe->queue[IPU3_CSS_QUEUE_IN].width_pad; - cfg_iter->input_info.format = - css_pipe->queue[IPU3_CSS_QUEUE_IN].css_fmt->frame_format; - cfg_iter->input_info.raw_bit_depth = - css_pipe->queue[IPU3_CSS_QUEUE_IN].css_fmt->bit_depth; - cfg_iter->input_info.raw_bayer_order = - css_pipe->queue[IPU3_CSS_QUEUE_IN].css_fmt->bayer_order; - cfg_iter->input_info.raw_type = IMGU_ABI_RAW_TYPE_BAYER; - - cfg_iter->internal_info.res.width = css_pipe->rect[IPU3_CSS_RECT_BDS].width; - cfg_iter->internal_info.res.height = - css_pipe->rect[IPU3_CSS_RECT_BDS].height; - cfg_iter->internal_info.padded_width = bds_width_pad; - cfg_iter->internal_info.format = - css_pipe->queue[IPU3_CSS_QUEUE_OUT].css_fmt->frame_format; - cfg_iter->internal_info.raw_bit_depth = - css_pipe->queue[IPU3_CSS_QUEUE_OUT].css_fmt->bit_depth; - cfg_iter->internal_info.raw_bayer_order = - css_pipe->queue[IPU3_CSS_QUEUE_OUT].css_fmt->bayer_order; - cfg_iter->internal_info.raw_type = IMGU_ABI_RAW_TYPE_BAYER; - - cfg_iter->output_info.res.width = - css_pipe->queue[IPU3_CSS_QUEUE_OUT].fmt.mpix.width; - cfg_iter->output_info.res.height = - css_pipe->queue[IPU3_CSS_QUEUE_OUT].fmt.mpix.height; - cfg_iter->output_info.padded_width = - css_pipe->queue[IPU3_CSS_QUEUE_OUT].width_pad; - cfg_iter->output_info.format = - css_pipe->queue[IPU3_CSS_QUEUE_OUT].css_fmt->frame_format; - cfg_iter->output_info.raw_bit_depth = -
[driver-core:driver-core-testing 84/85] drivers/s390/cio/device.c:1660:9: warning: assignment discards 'const' qualifier from pointer target type
tree: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/driver-core.git driver-core-testing head: 65b66682344a15ba2069d4dd8d0cc39cc3aed7e9 commit: 92ce7e83b4e5c86687d748ba53cb755acdce1256 [84/85] driver_find_device: Unify the match function with class_find_device() config: s390-debug_defconfig (attached as .config) compiler: s390-linux-gcc (GCC) 7.4.0 reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross git checkout 92ce7e83b4e5c86687d748ba53cb755acdce1256 # save the attached .config to linux build tree GCC_VERSION=7.4.0 make.cross ARCH=s390 If you fix the issue, kindly add following tag Reported-by: kbuild test robot All warnings (new ones prefixed by >>): drivers/s390/cio/device.c: In function '__ccwdev_check_busid': >> drivers/s390/cio/device.c:1660:9: warning: assignment discards 'const' >> qualifier from pointer target type [-Wdiscarded-qualifiers] bus_id = id; ^ -- drivers/s390/cio/ccwgroup.c: In function '__ccwgroupdev_check_busid': >> drivers/s390/cio/ccwgroup.c:613:17: warning: initialization discards 'const' >> qualifier from pointer target type [-Wdiscarded-qualifiers] char *bus_id = id; ^~ vim +/const +1660 drivers/s390/cio/device.c ^1da177e Linus Torvalds 2005-04-16 1651 ^1da177e Linus Torvalds 2005-04-16 1652 /* ^1da177e Linus Torvalds 2005-04-16 1653 * get ccw_device matching the busid, but only if owned by cdrv ^1da177e Linus Torvalds 2005-04-16 1654 */ b0744bd2 Cornelia Huck2005-06-25 1655 static int 92ce7e83 Suzuki K Poulose 2019-06-14 1656 __ccwdev_check_busid(struct device *dev, const void *id) b0744bd2 Cornelia Huck2005-06-25 1657 { b0744bd2 Cornelia Huck2005-06-25 1658 char *bus_id; b0744bd2 Cornelia Huck2005-06-25 1659 12975aef Cornelia Huck2006-10-11 @1660 bus_id = id; b0744bd2 Cornelia Huck2005-06-25 1661 98df67b3 Kay Sievers 2008-12-25 1662 return (strcmp(bus_id, dev_name(dev)) == 0); b0744bd2 Cornelia Huck2005-06-25 1663 } b0744bd2 Cornelia Huck2005-06-25 1664 :: The code at line 1660 was first introduced by commit :: 12975aef62836e9f3e179afaaded8045f8a25ac4 [S390] cio: remove casts from/to (void *). :: TO: Cornelia Huck :: CC: Martin Schwidefsky --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: application/gzip ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v4 0/3] [v4.9.y] coredump: fix race condition between mmget_not_zero()/get_task_mm() and core dumping
On 25/06/19, 1:51 AM, "Sasha Levin" wrote: > On Tue, Jun 25, 2019 at 02:33:06AM +0530, Ajay Kaher wrote: > > coredump: fix race condition between mmget_not_zero()/get_task_mm() > > and core dumping > > > > [PATCH v4 1/3]: > > Backporting of commit 04f5866e41fb70690e28397487d8bd8eea7d712a upstream. > > > > [PATCH v4 2/3]: > > Extension of commit 04f5866e41fb to fix the race condition between > > get_task_mm() and core dumping for IB->mlx4 and IB->mlx5 drivers. > > > > [PATCH v4 3/3] > > Backporting of commit 59ea6d06cfa9247b586a695c21f94afa7183af74 upstream. > > > > [diff from v3]: > > - added [PATCH v4 3/3] > Why do all the patches have the same subject line? Thanks for catching this. I will correct in next version of these patches, along with review comments if any. > I guess it's correct for the first one, but can you explain what's up > with #2 and #3? > > If the second one isn't upstream, please explain in detail why not and > how 4.9 differs from upstream so that it requires a custom backport. #2 applied to 4.14.y: https://git.kernel.org/pub/scm/linux/kernel/git/stable/stable-queue.git/tree/queue-4.14/infiniband-fix-race-condition-between-infiniband-mlx4-mlx5-driver-and-core-dumping.patch?id=e4041a3f6b569140549fe7b41ed527c5c1e38ec9 And then to 4.9.y (some part as requires). 4.18 and onwards doesn't have mmap_sem locking in mlx4 and mlx5, so no need of #2 in 4.18 and onwards. > The third one just looks like a different patch altogether with a wrong > subject line? #3 was in discussion here (during v1), so added here. > -- > Thanks, > Sasha ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: comedi: use dma_mmap_coherent for DMA-able buffer mmap
On 25/06/2019 13:06, Christoph Hellwig wrote: From the DMA point of view this looks good: Reviewed-by: Christoph Hellwig Thanks! I still think that doing that SetPageReserved + remap_pfn_range dance for the normal memory allocations is a bad idea. Just use vm_insert_page on the page, in which case it doesn't need to be marked as Reserved. Thanks for the hint. I'll take a look at that. -- -=( Ian Abbott || Web: www.mev.co.uk )=- -=( MEV Ltd. is a company registered in England & Wales. )=- -=( Registered number: 02862268. Registered address:)=- -=( 15 West Park Road, Bramhall, STOCKPORT, SK7 3JZ, UK. )=- ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/1] staging: media: fix style problem
On Tue, 2019-06-25 at 09:17 +0200, Hans Verkuil wrote: > On 6/21/19 8:39 AM, Aliasgar Surti wrote: > > From: Aliasgar Surti > > > > checkpatch reported "WARNING: line over 80 characters". > > This patch fixes the warning for file soc_camera/soc_ov5642.c > > FYI: we're not accepting patches for staging/media/soc_camera: these > are obsolete and broken drivers. Then mark the MAINTAINERS entry as Orphan / Obsolete It's currently: SOC-CAMERA V4L2 SUBSYSTEM L: linux-me...@vger.kernel.org T: git git://linuxtv.org/media_tree.git S: Orphan F: include/media/soc_camera.h F: drivers/staging/media/soc_camera/ ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: comedi: use dma_mmap_coherent for DMA-able buffer mmap
On 25/06/2019 12:47, Dan Carpenter wrote: On Tue, Jun 25, 2019 at 12:26:59PM +0100, Ian Abbott wrote: drivers/staging/comedi/comedi_buf.c | 150 ++- drivers/staging/comedi/comedi_fops.c | 39 --- 2 files changed, 125 insertions(+), 64 deletions(-) diff --git a/drivers/staging/comedi/comedi_buf.c b/drivers/staging/comedi/comedi_buf.c index d2c8cc72a99d..3ef3ddabf139 100644 --- a/drivers/staging/comedi/comedi_buf.c +++ b/drivers/staging/comedi/comedi_buf.c @@ -27,18 +27,19 @@ static void comedi_buf_map_kref_release(struct kref *kref) unsigned int i; if (bm->page_list) { - for (i = 0; i < bm->n_pages; i++) { - buf = >page_list[i]; - clear_bit(PG_reserved, - &(virt_to_page(buf->virt_addr)->flags)); - if (bm->dma_dir != DMA_NONE) { -#ifdef CONFIG_HAS_DMA - dma_free_coherent(bm->dma_hw_dev, - PAGE_SIZE, - buf->virt_addr, - buf->dma_addr); -#endif - } else { + if (bm->dma_dir != DMA_NONE) { + /* +* DMA buffer was allocated as a single block. +* Address is in page_list[0]. +*/ + buf = >page_list[0]; + dma_free_coherent(bm->dma_hw_dev, + PAGE_SIZE * bm->n_pages, + buf->virt_addr, buf->dma_addr); + } else { + for (i = 0; i < bm->n_pages; i++) { + buf = >page_list[i]; + ClearPageReserved(virt_to_page(buf->virt_addr)); I think we need a NULL check here: if (!buf->virt_addr) continue; free_page((unsigned long)buf->virt_addr); } } Hi Dan, I don't think that is strictly required because bm->n_pages gets set to the number of successfully allocated pages (not the number of required pages) by comedi_buf_map_alloc(): > + for (i = 0; i < n_pages; i++) { > + buf = >page_list[i]; > + buf->virt_addr = (void *)get_zeroed_page(GFP_KERNEL); > + if (!buf->virt_addr) > + break; > + > + SetPageReserved(virt_to_page(buf->virt_addr)); > + } > + > + bm->n_pages = i; Here! ^ > + if (i < n_pages) > + goto err; > +err: > + comedi_buf_map_put(bm); > + return NULL; -- -=( Ian Abbott || Web: www.mev.co.uk )=- -=( MEV Ltd. is a company registered in England & Wales. )=- -=( Registered number: 02862268. Registered address:)=- -=( 15 West Park Road, Bramhall, STOCKPORT, SK7 3JZ, UK. )=- ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: comedi: use dma_mmap_coherent for DMA-able buffer mmap
On Tue, Jun 25, 2019 at 02:21:41PM +0100, Ian Abbott wrote: > On 25/06/2019 12:47, Dan Carpenter wrote: > > On Tue, Jun 25, 2019 at 12:26:59PM +0100, Ian Abbott wrote: > > > + } else { > > > + for (i = 0; i < bm->n_pages; i++) { > > > + buf = >page_list[i]; > > > + ClearPageReserved(virt_to_page(buf->virt_addr)); > > > > I think we need a NULL check here: > > > > if (!buf->virt_addr) > > continue; > > > > > free_page((unsigned > > > long)buf->virt_addr); > > > } > > > } > > Hi Dan, I don't think that is strictly required because bm->n_pages gets set > to the number of successfully allocated pages (not the number of required > pages) by comedi_buf_map_alloc(): > > > + for (i = 0; i < n_pages; i++) { > > + buf = >page_list[i]; > > + buf->virt_addr = (void *)get_zeroed_page(GFP_KERNEL); > > + if (!buf->virt_addr) > > + break; > > + > > + SetPageReserved(virt_to_page(buf->virt_addr)); > > + } > > + > > + bm->n_pages = i; > > Here! ^ > Oh, yeah. I misread. Sorry for the noise. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 0/4] staging: mt7621-pci: Handle minor issues
Hi Greg, On Tue, Jun 25, 2019 at 7:18 AM Sergio Paracuellos wrote: > > Hi Greg, > > On Tue, Jun 25, 2019 at 7:10 AM Greg Ungerer wrote: > > > > Hi Sergio, > > > > On 23/6/19 3:58 pm, Sergio Paracuellos wrote: > > > On Sun, Jun 23, 2019 at 4:15 AM Brett Neumeier > > > wrote: > > >> On Fri, Jun 21, 2019 at 7:35 AM Greg Ungerer wrote: > > >>> On 21/6/19 4:15 pm, Sergio Paracuellos wrote: > > This patch series properly handle minor issues in this driver. These > > are: > > * Disable pcie port clock on pci dirver instead of doing it in the phy > > driver. The pci driver is the correct place to do this. > > * Add a missing call to phy_exit function to properly handle the > > function > > 'mt7621_pcie_init_port' error path. > > * Move driver to init in a later stage using 'module_init' instead of > > using > > 'arch_initcall'. > > > > Patches are only compile-tested. It would be awasome to be tested > > before applied > > them (mainly the change to 'module_init' stuff). > > >> > > >> I have similar results to Greg -- on GnuBee PC1 and PC2, six boot > > >> attempts each on a kernel built from staging-next, I have four hangs > > >> and eight successful boots. The hanging patterns are similar to > > >> previous results. If the full boot logs would be helpful let me know, > > >> I can provide them. > > > > > > Thanks for letting me know. One thing we can try is check init order > > > in v4.20 kernel. Can you please try to compile pci driver for the > > > kernel v4.20 tag changing > > > driver's last line 'arch_initcall' into 'module_init'? Just to know if > > > at that working driver putting all the stuff in a later stage stills > > > work as expected. > > > > > > Full dmesg's of this v4.20 wih the change would be helpful. > > > > Not exactly what you asked for, but just as useful I think. > > I have a linux-5.1.14 kernel with the linux-4.20 pci-mt7621.c > > driver in place. That works great, never hangs, always probes > > PCI and works. > > > > If I change the pci-mt7621.c arch_initcall() to module_init(), > > then I see the PCI probe happen much latter in boot. Running this > > I have booted about 15 times, no hangs, PCI bus probed and working > > on every boot. > > Perfect. That is exactly what I wanted to know. Thanks for testing this. > > > > > Boot log below. > > > > Regards > > Greg Can you please test the following change? diff --git a/drivers/staging/mt7621-pci/pci-mt7621.c b/drivers/staging/mt7621-pci/pci-mt7621.c index a981f4f0ed03..9ae14f722c91 100644 --- a/drivers/staging/mt7621-pci/pci-mt7621.c +++ b/drivers/staging/mt7621-pci/pci-mt7621.c @@ -41,8 +41,8 @@ /* MediaTek specific configuration registers */ #define PCIE_FTS_NUM 0x70c -#define PCIE_FTS_NUM_MASK GENMASK(15, 8) -#define PCIE_FTS_NUM_L0(x) ((x) & 0xff << 8) +#define PCIE_FTS_NUM_MASK GENMASK(7, 0) +#define PCIE_FTS_NUM_L0(x) ((x) << 8) /* rt_sysc_membase relative registers */ #define RALINK_CLKCFG1 0x30 @@ -540,7 +540,7 @@ static void mt7621_pcie_enable_ports(struct mt7621_pcie *pcie) write_config(pcie, slot, PCI_COMMAND, val); /* configure RC FTS number to 250 when it leaves L0s */ val = read_config(pcie, slot, PCIE_FTS_NUM); - val &= ~PCIE_FTS_NUM_MASK; + val &= ~(PCIE_FTS_NUM_MASK) << 8; val |= PCIE_FTS_NUM_L0(0x50); write_config(pcie, slot, PCIE_FTS_NUM, val); } Best regards, Sergio Paracuellos > > > > Linux version 5.1.14 (gerg@goober) (gcc version 5.4.0 (GCC)) #2 SMP Tue Jun > > 25 14:34:27 AEST 2019 > > SoC Type: MediaTek MT7621 ver:1 eco:3 > > printk: bootconsole [early0] enabled > > CPU0 revision is: 0001992f (MIPS 1004Kc) > > OF: fdt: No chosen node found, continuing without > > MIPS: machine is Digi EX15 > > Determined physical RAM map: > > memory: 1000 @ (usable) > > Initrd not found or empty - disabling initrd > > VPE topology {2,2} total 4 > > Primary instruction cache 32kB, VIPT, 4-way, linesize 32 bytes. > > Primary data cache 32kB, 4-way, PIPT, no aliases, linesize 32 bytes > > MIPS secondary cache 256kB, 8-way, linesize 32 bytes. > > Zone ranges: > >Normal [mem 0x-0x0fff] > > Movable zone start for each node > > Early memory node ranges > >node 0: [mem 0x-0x0fff] > > Initmem setup node 0 [mem 0x-0x0fff] > > random: get_random_bytes called from start_kernel+0xb0/0x51c with > > crng_init=0 > > percpu: Embedded 15 pages/cpu s30144 r8192 d23104 u61440 > > Built 1 zonelists, mobility grouping on. Total pages: 65024 > > Kernel command line: console=ttyS0,115200 ubi.mtd=3 root=/dev/mtdblock5 > > bootpart=a > > Dentry cache hash table entries: 32768 (order: 5, 131072 bytes) > > Inode-cache hash table entries: 16384
Re: [PATCH] media: schedule removal for legacy staging drivers
On Tue, Jun 25, 2019 at 08:48:26AM -0300, Mauro Carvalho Chehab wrote: > Keeping legacy problematic code forever is not a good idea. > > So, let's schedule a date for those legacy stuff to rest in piece. > > If someone wants to steps up and take them from the staging ostracism > and do give them a rejuvenation shower in order to address the > isues pointed on their TODO lists, be our guest! > > Signed-off-by: Mauro Carvalho Chehab Acked-by: Sakari Ailus -- Sakari Ailus sakari.ai...@linux.intel.com ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH V5 10/16] s390: zfcp_fc: use sg helper to operate scatterlist
Hi Ming, I don't mind doing this change for zfcp. However, I'm having doubts regarding the rationale in the commit description. If I understood your patch series correctly from its cover letter (would have been nice to copy the SCSI MQ detail part of its core statement in (one of) the patches so it would make its way into git log), you plan to make a change to scatterlist allocations *in SCSI MQ*. All zfcp changes in this patch set only refer to zfcp-internal remote port discovery, i.e. they neither come from SCSI (internally) nor from block(mq). zfcp_fc.h: /** * struct zfcp_fc_req - Container for FC ELS and CT requests sent from zfcp * @ct_els: data required for issuing fsf command * @sg_req: scatterlist entry for request data, refers to embedded @u submember * @sg_rsp: scatterlist entry for response data, refers to embedded @u submember * @u: request and response specific data * @u.gpn_ft: GPN_FT specific data * @u.gpn_ft.sg_rsp2: GPN_FT response, not embedded here, allocated elsewhere * @u.gpn_ft.req: GPN_FT request */ struct zfcp_fc_req { struct zfcp_fsf_ct_els ct_els; struct scatterlist sg_req; struct scatterlist sg_rsp; union { struct { struct scatterlist sg_rsp2[ZFCP_FC_GPN_FT_NUM_BUFS - 1]; struct zfcp_fc_gpn_ft_req req; } gpn_ft; } u; }; So this should be guaranteed to be a linear unchained scatterlist, independently of your SCSI (or block) changes. Note: Only remote port discovery also uses u.gpn_ft.sg_rsp2 instead of just sg_rsp. zfcp_fc_scan_ports(work) zfcp_fc_alloc_sg_env(buf_num) zfcp_fc_sg_setup_table(_req->sg_rsp, buf_num) (In fact it's somewhat intricate, because it actually uses sg_rsp and seems to rely on the fact that the subsequent sg_rsp2[] gives enough contiguous memory to hold buf_num linear scatterlist entries starting with field offset sg_rsp.) The other cases use single element and thus linear unchained scatterlist with sg_rsp (and all cases use sg_req): Finding symbol: sg_init_one Database directory: /home/maier/docs/zfcp/tuxmaker/linux/drivers/s390/scsi/ --- *** zfcp_dbf.c: zfcp_dbf_san_in_els[601] sg_init_one(, srb->payload.data, length); // above tracing part is unrelated to all of scsi/block/zfcp-internal-ct/els *** zfcp_fc.c: zfcp_fc_ns_gid_pn_request[388] sg_init_one(_req->sg_req, gid_pn_req, sizeof(*gid_pn_req)); zfcp_fc_ns_gid_pn_request[389] sg_init_one(_req->sg_rsp, gid_pn_rsp, sizeof(*gid_pn_rsp)); zfcp_fc_adisc[546] sg_init_one(_req->sg_req, _req->u.adisc.req, zfcp_fc_adisc[548] sg_init_one(_req->sg_rsp, _req->u.adisc.rsp, zfcp_fc_alloc_sg_env[668] sg_init_one(_req->sg_req, _req->u.gpn_ft.req, zfcp_fc_gspn[841] sg_init_one(_req->sg_req, gspn_req, sizeof(*gspn_req)); zfcp_fc_gspn[842] sg_init_one(_req->sg_rsp, gspn_rsp, sizeof(*gspn_rsp)); zfcp_fc_rspn[889] sg_init_one(_req->sg_req, rspn_req, sizeof(*rspn_req)); zfcp_fc_rspn[890] sg_init_one(_req->sg_rsp, rspn_rsp, sizeof(*rspn_rsp)); --- I/O requests from SCSI (MQ) coming through queuecommand have already been safe for non-linear chained scatterlists in zfcp: zfcp_scsi_queuecommand() zfcp_fsf_fcp_cmnd() zfcp_qdio_sbals_from_sg(qdio, >qdio_req, scsi_sglist(scsi_cmnd)) for (; sg; sg = sg_next(sg)) { I/O requests from the block layer coming through BSG should also have already been safe for non-linear chained scatterlists in zfcp: zfcp_fc_exec_bsg_job() zfcp_fc_exec_els_job() zfcp_fsf_send_els() zfcp_fsf_setup_ct_els() zfcp_fsf_setup_ct_els_sbals(req, sg_req, sg_resp) //depending on hardware features, translate sg into HW control blocks if (zfcp_adapter_multi_buffer_active()) zfcp_qdio_sbals_from_sg() //for req, see above return 0 /* use single, unchained SBAL if it can hold the request */ if (zfcp_qdio_sg_one_sbale(sg_req) && zfcp_qdio_sg_one_sbale(sg_resp)) zfcp_fsf_setup_ct_els_unchained() //single element for req return 0 if (!(feat & FSF_FEATURE_ELS_CT_CHAINED_SBALS)) return -EOPNOTSUPP; zfcp_qdio_sbals_from_sg() for req, see above OR zfcp_fc_exec_ct_job() zfcp_fsf_send_ct() zfcp_fsf_setup_ct_els() //see above If I was not mistaken above, the following could be more descriptive parts of a patch/commit description, with hopefully less confusion for anyone having to look at zfcp git history a few weeks/months/years from now: "While not required for this SCSI MQ change regarding scatterlist allocation, change all other scatterlist iterators in zfcp
Re: [PATCH] staging: comedi: use dma_mmap_coherent for DMA-able buffer mmap
>From the DMA point of view this looks good: Reviewed-by: Christoph Hellwig I still think that doing that SetPageReserved + remap_pfn_range dance for the normal memory allocations is a bad idea. Just use vm_insert_page on the page, in which case it doesn't need to be marked as Reserved. On Tue, Jun 25, 2019 at 12:26:59PM +0100, Ian Abbott wrote: > Comedi's acquisition buffer allocation code can allocate the buffer from > normal kernel memory or from DMA coherent memory depending on the > `dma_async_dir` value in the comedi subdevice. (A value of `DMA_NONE` > causes the buffer to be allocated from normal kernel memory. Other > values cause the buffer to be allocated from DMA coherent memory.) The > buffer currently consists of a bunch of page-sized blocks that are > vmap'ed into a contiguous range of virtual addresses. The pages are also > mmap'able into user address space. For DMA'able buffers, these > page-sized blocks are allocated by `dma_alloc_coherent()`. > > For DMA-able buffers, the DMA API is currently abused in various ways, > the most serious abuse being the calling of `virt_to_page()` on the > blocks allocated by `dma_alloc_coherent()` and passing those pages to > `vmap()` (for mapping to the kernels vmalloc address space) and via > `page_to_pfn()` to `remap_pfn_range()` (for mmap'ing to user space). it > also uses the `__GFP_COMP` flag when allocating the blocks, and marks > the allocated pages as reserved (which is unnecessary for DMA coherent > allocations). > > The code can be changed to get rid of the vmap'ed address altogether if > necessary, since there are only a few places in the comedi code that use > the vmap'ed address directly and we also keep a list of the kernel > addresses for the individual pages prior to the vmap operation. This > would add some run-time overhead to buffer accesses. The real killer is > the mmap operation. > > For mmap, the address range specified in the VMA needs to be mmap'ed to > the individually allocated page-sized blocks. That is not a problem > when the pages are allocated from normal kernel memory as the individual > pages can be remapped by `remap_pfn_range()`, but it is a problem when > the page-sized blocks are allocated by `dma_alloc_coherent()` because > the DMA API currently has no support for splitting a VMA across multiple > blocks of DMA coherent memory (or rather, no support for mapping part of > a VMA range to a single block of DMA coherent memory). > > In order to comply with the DMA API and allow the buffer to be mmap'ed, > the buffer needs to be allocated as a single block by a single call to > `dma_alloc_coherent()`, freed by a single call to `dma_free_coherent()`, > and mmap'ed to user space by a single call to `dma_mmap_coherent()`. > This patch changes the buffer allocation, freeing, and mmap'ing code to > do that, with the unfortunate consequence that buffer allocation is more > likely to fail. It also no longer uses the `__GFP_COMP` flag when > allocating DMA coherent memory, no longer marks the > allocated pages of DMA coherent memory as reserved, and no longer vmap's > the DMA coherent memory pages (since they are contiguous anyway). > > Cc: Christoph Hellwig > Signed-off-by: Ian Abbott > --- > drivers/staging/comedi/comedi_buf.c | 150 ++- > drivers/staging/comedi/comedi_fops.c | 39 --- > 2 files changed, 125 insertions(+), 64 deletions(-) > > diff --git a/drivers/staging/comedi/comedi_buf.c > b/drivers/staging/comedi/comedi_buf.c > index d2c8cc72a99d..3ef3ddabf139 100644 > --- a/drivers/staging/comedi/comedi_buf.c > +++ b/drivers/staging/comedi/comedi_buf.c > @@ -27,18 +27,19 @@ static void comedi_buf_map_kref_release(struct kref *kref) > unsigned int i; > > if (bm->page_list) { > - for (i = 0; i < bm->n_pages; i++) { > - buf = >page_list[i]; > - clear_bit(PG_reserved, > - &(virt_to_page(buf->virt_addr)->flags)); > - if (bm->dma_dir != DMA_NONE) { > -#ifdef CONFIG_HAS_DMA > - dma_free_coherent(bm->dma_hw_dev, > - PAGE_SIZE, > - buf->virt_addr, > - buf->dma_addr); > -#endif > - } else { > + if (bm->dma_dir != DMA_NONE) { > + /* > + * DMA buffer was allocated as a single block. > + * Address is in page_list[0]. > + */ > + buf = >page_list[0]; > + dma_free_coherent(bm->dma_hw_dev, > + PAGE_SIZE * bm->n_pages, > + buf->virt_addr, buf->dma_addr); > + } else { > + for (i = 0; i < bm->n_pages; i++) { > + buf =
Re: [PATCH 6/8] staging: kpc2000: introduce 'unsigned int'
On Tue, Jun 25, 2019 at 01:27:17PM +0200, Fabian Krueger wrote: > Replaced 'unsigned' with it's equivalent 'unsigned int' to reduce > confusion while reading the code. > > Signed-off-by: Fabian Krueger > Signed-off-by: Michael Scheiderer > --- > drivers/staging/kpc2000/kpc2000_spi.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/staging/kpc2000/kpc2000_spi.c > b/drivers/staging/kpc2000/kpc2000_spi.c > index 79d7c44315e8..732254e9b261 100644 > --- a/drivers/staging/kpc2000/kpc2000_spi.c > +++ b/drivers/staging/kpc2000/kpc2000_spi.c > @@ -337,7 +337,7 @@ kp_spi_transfer_one_message(struct spi_master *master, > struct spi_message *m) > list_for_each_entry(transfer, >transfers, transfer_list) { > const void *tx_buf = transfer->tx_buf; > void *rx_buf = transfer->rx_buf; > - unsignedlen = transfer->len; > + unsigned int len = transfer->len; This white space isn't correct. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 4/8] staging: kpc2000: add spaces
On Tue, Jun 25, 2019 at 01:27:15PM +0200, Fabian Krueger wrote: > Added spaces on the left side of parenthesis and on both sides of binary > operators. > This refactoring makes the code more readable. > > Signed-off-by: Fabian Krueger > Signed-off-by: Michael Scheiderer > --- > drivers/staging/kpc2000/kpc2000_spi.c | 15 ++- > 1 file changed, 6 insertions(+), 9 deletions(-) > > diff --git a/drivers/staging/kpc2000/kpc2000_spi.c > b/drivers/staging/kpc2000/kpc2000_spi.c > index 253a9c150d33..8f56886f4673 100644 > --- a/drivers/staging/kpc2000/kpc2000_spi.c > +++ b/drivers/staging/kpc2000/kpc2000_spi.c > @@ -192,9 +192,8 @@ kp_spi_read_reg(struct kp_spi_controller_state *cs, int > idx) > u64 val; > > addr += idx; > - if ((idx == KP_SPI_REG_CONFIG) && (cs->conf_cache >= 0)){ > + if ((idx == KP_SPI_REG_CONFIG) && (cs->conf_cache >= 0)) > return cs->conf_cache; > - } This doesn't match the patch description. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/8] staging: kpc2000: add line breaks
On Tue, Jun 25, 2019 at 01:27:12PM +0200, Fabian Krueger wrote: > diff --git a/drivers/staging/kpc2000/kpc2000_spi.c > b/drivers/staging/kpc2000/kpc2000_spi.c > index c3e5c1848f53..7ed0fb6b4abb 100644 > --- a/drivers/staging/kpc2000/kpc2000_spi.c > +++ b/drivers/staging/kpc2000/kpc2000_spi.c > @@ -30,18 +30,46 @@ > #include "kpc.h" > > static struct mtd_partition p2kr0_spi0_parts[] = { > - { .name = "SLOT_0", .size = 7798784,.offset = 0, > }, > - { .name = "SLOT_1", .size = 7798784,.offset = > MTDPART_OFS_NXTBLK}, > - { .name = "SLOT_2", .size = 7798784,.offset = > MTDPART_OFS_NXTBLK}, > - { .name = "SLOT_3", .size = 7798784,.offset = > MTDPART_OFS_NXTBLK}, > - { .name = "CS0_EXTRA", .size = MTDPART_SIZ_FULL, .offset = > MTDPART_OFS_NXTBLK}, > + {.name = "SLOT_0", > + .size = 7798784, > + .offset = 0,}, > + > + {.name = "SLOT_1", > + .size = 7798784, > + .offset = MTDPART_OFS_NXTBLK}, > + > + {.name = "SLOT_2", > + .size = 7798784, > + .offset = MTDPART_OFS_NXTBLK}, > + > + {.name = "SLOT_3", > + .size = 7798784, > + .offset = MTDPART_OFS_NXTBLK}, > + > + {.name = "CS0_EXTRA", > + .size = MTDPART_SIZ_FULL, > + .offset = MTDPART_OFS_NXTBLK}, > }; The original is fine/better. > @@ -215,7 +244,9 @@ kp_spi_txrx_pio(struct spi_device *spidev, struct > spi_transfer *transfer) > for (i = 0 ; i < c ; i++) { > char val = *tx++; > > - if (kp_spi_wait_for_reg_bit(cs, KP_SPI_REG_STATUS, > KP_SPI_REG_STATUS_TXS) < 0) { > + if (kp_spi_wait_for_reg_bit(cs, KP_SPI_REG_STATUS, > + KP_SPI_REG_STATUS_TXS) > + < 0) { I don't like how the < 0 is on the next line. It would be better to introduce an "int ret;" ret = kp_spi_wait_for_reg_bit(cs, KP_SPI_REG_STATUS, KP_SPI_REG_STATUS_TXS); if (ret < 0) goto out; > goto out; > } > > @@ -317,7 +353,8 @@ kp_spi_transfer_one_message(struct spi_master *master, > struct spi_message *m) > dev_dbg(kpspi->dev, " transfer -EINVAL\n"); > return -EINVAL; > } > - if (transfer->speed_hz && (transfer->speed_hz < (KP_SPI_CLK >> > 15))) { > + if (transfer->speed_hz && (transfer->speed_hz < > +(KP_SPI_CLK >> 15))) { Move the whole conition down: if (transfer->speed_hz && transfer->speed_hz < (KP_SPI_CLK >> 15)) { regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: comedi: use dma_mmap_coherent for DMA-able buffer mmap
On Tue, Jun 25, 2019 at 12:26:59PM +0100, Ian Abbott wrote: > drivers/staging/comedi/comedi_buf.c | 150 ++- > drivers/staging/comedi/comedi_fops.c | 39 --- > 2 files changed, 125 insertions(+), 64 deletions(-) > > diff --git a/drivers/staging/comedi/comedi_buf.c > b/drivers/staging/comedi/comedi_buf.c > index d2c8cc72a99d..3ef3ddabf139 100644 > --- a/drivers/staging/comedi/comedi_buf.c > +++ b/drivers/staging/comedi/comedi_buf.c > @@ -27,18 +27,19 @@ static void comedi_buf_map_kref_release(struct kref *kref) > unsigned int i; > > if (bm->page_list) { > - for (i = 0; i < bm->n_pages; i++) { > - buf = >page_list[i]; > - clear_bit(PG_reserved, > - &(virt_to_page(buf->virt_addr)->flags)); > - if (bm->dma_dir != DMA_NONE) { > -#ifdef CONFIG_HAS_DMA > - dma_free_coherent(bm->dma_hw_dev, > - PAGE_SIZE, > - buf->virt_addr, > - buf->dma_addr); > -#endif > - } else { > + if (bm->dma_dir != DMA_NONE) { > + /* > + * DMA buffer was allocated as a single block. > + * Address is in page_list[0]. > + */ > + buf = >page_list[0]; > + dma_free_coherent(bm->dma_hw_dev, > + PAGE_SIZE * bm->n_pages, > + buf->virt_addr, buf->dma_addr); > + } else { > + for (i = 0; i < bm->n_pages; i++) { > + buf = >page_list[i]; > + ClearPageReserved(virt_to_page(buf->virt_addr)); I think we need a NULL check here: if (!buf->virt_addr) continue; > free_page((unsigned long)buf->virt_addr); > } > } > @@ -57,7 +58,8 @@ static void __comedi_buf_free(struct comedi_device *dev, regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] media: schedule removal for legacy staging drivers
Keeping legacy problematic code forever is not a good idea. So, let's schedule a date for those legacy stuff to rest in piece. If someone wants to steps up and take them from the staging ostracism and do give them a rejuvenation shower in order to address the isues pointed on their TODO lists, be our guest! Signed-off-by: Mauro Carvalho Chehab --- drivers/staging/media/bcm2048/TODO | 6 ++ drivers/staging/media/davinci_vpfe/TODO | 7 +++ drivers/staging/media/omap4iss/TODO | 7 +++ drivers/staging/media/soc_camera/TODO | 9 + 4 files changed, 29 insertions(+) diff --git a/drivers/staging/media/bcm2048/TODO b/drivers/staging/media/bcm2048/TODO index 6bee2a2dad68..478984672c9b 100644 --- a/drivers/staging/media/bcm2048/TODO +++ b/drivers/staging/media/bcm2048/TODO @@ -1,3 +1,9 @@ +NOTE: + The bcm2048 driver fixes on this TODO were not addressed yet. + It has been a long time since the last related change. + Unless someone steps up addressing those, this driver is + scheduled to be removed for Kernel 5.4. + TODO: From the initial code review: diff --git a/drivers/staging/media/davinci_vpfe/TODO b/drivers/staging/media/davinci_vpfe/TODO index cc8bd9306f2a..9d4577a911c9 100644 --- a/drivers/staging/media/davinci_vpfe/TODO +++ b/drivers/staging/media/davinci_vpfe/TODO @@ -1,3 +1,10 @@ +NOTE: + The davinci_vpfe driver fixes on this TODO were not addressed yet. + It has been a long time since the last related change. + Unless someone steps up addressing those, this driver is + scheduled to be removed for Kernel 5.4. + + TODO (general): == diff --git a/drivers/staging/media/omap4iss/TODO b/drivers/staging/media/omap4iss/TODO index 4d220ef82653..fb90be3c1f32 100644 --- a/drivers/staging/media/omap4iss/TODO +++ b/drivers/staging/media/omap4iss/TODO @@ -1,3 +1,10 @@ +NOTE: + The omap4iss driver fixes on this TODO were not addressed yet. + It has been a long time since the last related change. + Unless someone steps up addressing those, this driver is + scheduled to be removed for Kernel 5.4. + + * Fix FIFO/buffer overflows and underflows * Replace dummy resizer code with a real implementation * Fix checkpatch errors and warnings diff --git a/drivers/staging/media/soc_camera/TODO b/drivers/staging/media/soc_camera/TODO index 932af6443b67..677dcdc1de61 100644 --- a/drivers/staging/media/soc_camera/TODO +++ b/drivers/staging/media/soc_camera/TODO @@ -1,3 +1,12 @@ +NOTE: + The old drivers that depends on SoC camera framework require + conversion. We're not accepting any patches that are doing just + checkpatch or style fixes before such conversion. + + If nobody steps up addressing those, this driver is scheduled to be + removed for Kernel 5.5. + + The SoC camera framework is obsolete and scheduled for removal in the near future. Developers are encouraged to convert the drivers to use the regular V4L2 API if these drivers are still needed (and if someone has the -- 2.21.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 6/8] staging: kpc2000: introduce 'unsigned int'
Replaced 'unsigned' with it's equivalent 'unsigned int' to reduce confusion while reading the code. Signed-off-by: Fabian Krueger Signed-off-by: Michael Scheiderer --- drivers/staging/kpc2000/kpc2000_spi.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/staging/kpc2000/kpc2000_spi.c b/drivers/staging/kpc2000/kpc2000_spi.c index 79d7c44315e8..732254e9b261 100644 --- a/drivers/staging/kpc2000/kpc2000_spi.c +++ b/drivers/staging/kpc2000/kpc2000_spi.c @@ -337,7 +337,7 @@ kp_spi_transfer_one_message(struct spi_master *master, struct spi_message *m) list_for_each_entry(transfer, >transfers, transfer_list) { const void *tx_buf = transfer->tx_buf; void *rx_buf = transfer->rx_buf; - unsignedlen = transfer->len; + unsigned int len = transfer->len; if (transfer->speed_hz > KP_SPI_CLK || (len && !(rx_buf || tx_buf))) { @@ -383,7 +383,7 @@ kp_spi_transfer_one_message(struct spi_master *master, struct spi_message *m) /* transfer */ if (transfer->len) { unsigned int word_len = spidev->bits_per_word; - unsigned count; + unsigned int count; /* set up the transfer... */ sc.reg = kp_spi_read_reg(cs, KP_SPI_REG_CONFIG); -- 2.17.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 4/8] staging: kpc2000: add spaces
Added spaces on the left side of parenthesis and on both sides of binary operators. This refactoring makes the code more readable. Signed-off-by: Fabian Krueger Signed-off-by: Michael Scheiderer --- drivers/staging/kpc2000/kpc2000_spi.c | 15 ++- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/drivers/staging/kpc2000/kpc2000_spi.c b/drivers/staging/kpc2000/kpc2000_spi.c index 253a9c150d33..8f56886f4673 100644 --- a/drivers/staging/kpc2000/kpc2000_spi.c +++ b/drivers/staging/kpc2000/kpc2000_spi.c @@ -192,9 +192,8 @@ kp_spi_read_reg(struct kp_spi_controller_state *cs, int idx) u64 val; addr += idx; - if ((idx == KP_SPI_REG_CONFIG) && (cs->conf_cache >= 0)){ + if ((idx == KP_SPI_REG_CONFIG) && (cs->conf_cache >= 0)) return cs->conf_cache; - } val = readq(addr); return val; } @@ -255,10 +254,9 @@ kp_spi_txrx_pio(struct spi_device *spidev, struct spi_transfer *transfer) kp_spi_write_reg(cs, KP_SPI_REG_TXDATA, val); processed++; } - } - else if(rx) { + } else if (rx) { for (i = 0 ; i < c ; i++) { - char test=0; + char test = 0; kp_spi_write_reg(cs, KP_SPI_REG_TXDATA, 0x00); @@ -298,9 +296,8 @@ kp_spi_setup(struct spi_device *spidev) cs = spidev->controller_state; if (!cs) { cs = kzalloc(sizeof(*cs), GFP_KERNEL); - if(!cs) { + if (!cs) return -ENOMEM; - } cs->base = kpspi->base; cs->conf_cache = -1; spidev->controller_state = cs; @@ -467,7 +464,7 @@ kp_spi_probe(struct platform_device *pldev) int i; drvdata = pldev->dev.platform_data; - if (!drvdata){ + if (!drvdata) { dev_err(>dev, "kp_spi_probe: platform_data is NULL!\n"); return -ENODEV; } @@ -518,7 +515,7 @@ kp_spi_probe(struct platform_device *pldev) spi_new_device(master, &(table[i])); \ } - switch ((drvdata->card_id & 0x) >> 16){ + switch ((drvdata->card_id & 0x) >> 16) { case PCI_DEVICE_ID_DAKTRONICS_KADOKA_P2KR0: NEW_SPI_DEVICE_FROM_BOARD_INFO_TABLE(p2kr0_board_info); break; -- 2.17.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 3/8] staging: kpc2000: introduce usage of __packed
Replaced __attribute__((packed)) with __packed. Both ways of attributing are equivalent, but being shorter, __packed should be preferred. This refactoring makes the core more readable. Signed-off-by: Fabian Krueger Signed-off-by: Michael Scheiderer --- drivers/staging/kpc2000/kpc2000_spi.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/staging/kpc2000/kpc2000_spi.c b/drivers/staging/kpc2000/kpc2000_spi.c index e0e7703c6e53..253a9c150d33 100644 --- a/drivers/staging/kpc2000/kpc2000_spi.c +++ b/drivers/staging/kpc2000/kpc2000_spi.c @@ -141,7 +141,7 @@ struct kp_spi_controller_state { union kp_spi_config { /* use this to access individual elements */ - struct __attribute__((packed)) spi_config_bitfield { + struct __packed spi_config_bitfield { unsigned int pha : 1; /* spim_clk Phase */ unsigned int pol : 1; /* spim_clk Polarity */ unsigned int epol : 1; /* spim_csx Polarity */ @@ -160,7 +160,7 @@ union kp_spi_config { }; union kp_spi_status { - struct __attribute__((packed)) spi_status_bitfield { + struct __packed spi_status_bitfield { unsigned int rx: 1; /* Rx Status */ unsigned int tx: 1; /* Tx Status */ unsigned int eo: 1; /* End of Transfer */ @@ -175,7 +175,7 @@ union kp_spi_status { }; union kp_spi_ffctrl { - struct __attribute__((packed)) spi_ffctrl_bitfield { + struct __packed spi_ffctrl_bitfield { unsigned int ffstart : 1; /* FIFO Start */ unsigned int : 31; } bitfield; -- 2.17.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/8] staging: kpc2000: blank lines after declaration
After the declarations in a function, there should be a blank line, so that the declaration part is visibly separated from the rest. This refactoring makes the code more readable. Signed-off-by: Fabian Krueger Signed-off-by: Michael Scheiderer --- drivers/staging/kpc2000/kpc2000_spi.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/staging/kpc2000/kpc2000_spi.c b/drivers/staging/kpc2000/kpc2000_spi.c index 7ed0fb6b4abb..e0e7703c6e53 100644 --- a/drivers/staging/kpc2000/kpc2000_spi.c +++ b/drivers/staging/kpc2000/kpc2000_spi.c @@ -203,6 +203,7 @@ kp_spi_read_reg(struct kp_spi_controller_state *cs, int idx) kp_spi_write_reg(struct kp_spi_controller_state *cs, int idx, u64 val) { u64 __iomem *addr = cs->base; + addr += idx; writeq(val, addr); if (idx == KP_SPI_REG_CONFIG) @@ -214,6 +215,7 @@ kp_spi_wait_for_reg_bit(struct kp_spi_controller_state *cs, int idx, unsigned long bit) { unsigned long timeout; + timeout = jiffies + msecs_to_jiffies(1000); while (!(kp_spi_read_reg(cs, idx) & bit)) { if (time_after(jiffies, timeout)) { @@ -445,6 +447,7 @@ kp_spi_transfer_one_message(struct spi_master *master, struct spi_message *m) kp_spi_cleanup(struct spi_device *spidev) { struct kp_spi_controller_state *cs = spidev->controller_state; + if (cs) { kfree(cs); } @@ -536,6 +539,7 @@ kp_spi_probe(struct platform_device *pldev) kp_spi_remove(struct platform_device *pldev) { struct spi_master *master = platform_get_drvdata(pldev); + spi_unregister_master(master); return 0; } -- 2.17.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 5/8] staging: kpc2000: remove unnecessary brackets
Removed brackets on around one-lined if-cases. This refactoring makes the code more readable. Signed-off-by: Fabian Krueger Signed-off-by: Michael Scheiderer --- drivers/staging/kpc2000/kpc2000_spi.c | 27 +-- 1 file changed, 9 insertions(+), 18 deletions(-) diff --git a/drivers/staging/kpc2000/kpc2000_spi.c b/drivers/staging/kpc2000/kpc2000_spi.c index 8f56886f4673..79d7c44315e8 100644 --- a/drivers/staging/kpc2000/kpc2000_spi.c +++ b/drivers/staging/kpc2000/kpc2000_spi.c @@ -218,11 +218,10 @@ kp_spi_wait_for_reg_bit(struct kp_spi_controller_state *cs, int idx, timeout = jiffies + msecs_to_jiffies(1000); while (!(kp_spi_read_reg(cs, idx) & bit)) { if (time_after(jiffies, timeout)) { - if (!(kp_spi_read_reg(cs, idx) & bit)) { + if (!(kp_spi_read_reg(cs, idx) & bit)) return -ETIMEDOUT; - } else { + else return 0; - } } cpu_relax(); } @@ -331,9 +330,8 @@ kp_spi_transfer_one_message(struct spi_master *master, struct spi_message *m) cs = spidev->controller_state; /* reject invalid messages and transfers */ - if (list_empty(>transfers)) { + if (list_empty(>transfers)) return -EINVAL; - } /* validate input */ list_for_each_entry(transfer, >transfers, transfer_list) { @@ -391,17 +389,14 @@ kp_spi_transfer_one_message(struct spi_master *master, struct spi_message *m) sc.reg = kp_spi_read_reg(cs, KP_SPI_REG_CONFIG); /* ...direction */ - if (transfer->tx_buf) { + if (transfer->tx_buf) sc.bitfield.trm = KP_SPI_REG_CONFIG_TRM_TX; - } - else if (transfer->rx_buf) { + else if (transfer->rx_buf) sc.bitfield.trm = KP_SPI_REG_CONFIG_TRM_RX; - } /* ...word length */ - if (transfer->bits_per_word) { + if (transfer->bits_per_word) word_len = transfer->bits_per_word; - } sc.bitfield.wl = word_len-1; /* ...chip select */ @@ -420,9 +415,8 @@ kp_spi_transfer_one_message(struct spi_master *master, struct spi_message *m) } } - if (transfer->delay_usecs) { + if (transfer->delay_usecs) udelay(transfer->delay_usecs); - } } /* de-assert chip select to end the sequence */ @@ -445,9 +439,7 @@ kp_spi_cleanup(struct spi_device *spidev) { struct kp_spi_controller_state *cs = spidev->controller_state; - if (cs) { - kfree(cs); - } + kfree(cs); } /** @@ -489,9 +481,8 @@ kp_spi_probe(struct platform_device *pldev) kpspi->dev = >dev; master->num_chipselect = 4; - if (pldev->id != -1) { + if (pldev->id != -1) master->bus_num = pldev->id; - } r = platform_get_resource(pldev, IORESOURCE_MEM, 0); if (r == NULL) { -- 2.17.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 8/8] staging: kpc2000: remove needless 'break'
The unconditioned jump will prohibit to ever reach the break-statement. Deleting this needless statement, the code becomes more understandable. Signed-off-by: Fabian Krueger Signed-off-by: Michael Scheiderer --- drivers/staging/kpc2000/kpc2000_spi.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/staging/kpc2000/kpc2000_spi.c b/drivers/staging/kpc2000/kpc2000_spi.c index f258f369e065..e65f0c34db50 100644 --- a/drivers/staging/kpc2000/kpc2000_spi.c +++ b/drivers/staging/kpc2000/kpc2000_spi.c @@ -515,7 +515,6 @@ kp_spi_probe(struct platform_device *pldev) default: dev_err(>dev, "Unknown hardware, cant know what partition table to use!\n"); goto free_master; - break; } return status; -- 2.17.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/8] staging: kpc2000: add line breaks
To fix some checkpatch-warnings some lines of this module had to be shortened so that they do not exceed 80 characters per line. This refactoring makes the code more readable. Signed-off-by: Fabian Krueger Signed-off-by: Michael Scheiderer Cc: --- drivers/staging/kpc2000/kpc2000_spi.c | 77 --- 1 file changed, 58 insertions(+), 19 deletions(-) diff --git a/drivers/staging/kpc2000/kpc2000_spi.c b/drivers/staging/kpc2000/kpc2000_spi.c index c3e5c1848f53..7ed0fb6b4abb 100644 --- a/drivers/staging/kpc2000/kpc2000_spi.c +++ b/drivers/staging/kpc2000/kpc2000_spi.c @@ -30,18 +30,46 @@ #include "kpc.h" static struct mtd_partition p2kr0_spi0_parts[] = { - { .name = "SLOT_0", .size = 7798784,.offset = 0, }, - { .name = "SLOT_1", .size = 7798784,.offset = MTDPART_OFS_NXTBLK}, - { .name = "SLOT_2", .size = 7798784,.offset = MTDPART_OFS_NXTBLK}, - { .name = "SLOT_3", .size = 7798784,.offset = MTDPART_OFS_NXTBLK}, - { .name = "CS0_EXTRA", .size = MTDPART_SIZ_FULL, .offset = MTDPART_OFS_NXTBLK}, + {.name = "SLOT_0", +.size = 7798784, +.offset = 0,}, + + {.name = "SLOT_1", +.size = 7798784, +.offset = MTDPART_OFS_NXTBLK}, + + {.name = "SLOT_2", +.size = 7798784, +.offset = MTDPART_OFS_NXTBLK}, + + {.name = "SLOT_3", +.size = 7798784, +.offset = MTDPART_OFS_NXTBLK}, + + {.name = "CS0_EXTRA", +.size = MTDPART_SIZ_FULL, +.offset = MTDPART_OFS_NXTBLK}, }; static struct mtd_partition p2kr0_spi1_parts[] = { - { .name = "SLOT_4", .size = 7798784,.offset = 0, }, - { .name = "SLOT_5", .size = 7798784,.offset = MTDPART_OFS_NXTBLK}, - { .name = "SLOT_6", .size = 7798784,.offset = MTDPART_OFS_NXTBLK}, - { .name = "SLOT_7", .size = 7798784,.offset = MTDPART_OFS_NXTBLK}, - { .name = "CS1_EXTRA", .size = MTDPART_SIZ_FULL, .offset = MTDPART_OFS_NXTBLK}, + {.name = "SLOT_4", +.size = 7798784, +.offset = 0,}, + + {.name = "SLOT_5", +.size = 7798784, +.offset = MTDPART_OFS_NXTBLK}, + + {.name = "SLOT_6", +.size = 7798784, +.offset = MTDPART_OFS_NXTBLK}, + + {.name = "SLOT_7", +.size = 7798784, +.offset = MTDPART_OFS_NXTBLK}, + + {.name = "CS1_EXTRA", +.size = MTDPART_SIZ_FULL, +.offset = MTDPART_OFS_NXTBLK}, }; static struct flash_platform_data p2kr0_spi0_pdata = { @@ -182,7 +210,8 @@ kp_spi_write_reg(struct kp_spi_controller_state *cs, int idx, u64 val) } static int -kp_spi_wait_for_reg_bit(struct kp_spi_controller_state *cs, int idx, unsigned long bit) +kp_spi_wait_for_reg_bit(struct kp_spi_controller_state *cs, int idx, + unsigned long bit) { unsigned long timeout; timeout = jiffies + msecs_to_jiffies(1000); @@ -215,7 +244,9 @@ kp_spi_txrx_pio(struct spi_device *spidev, struct spi_transfer *transfer) for (i = 0 ; i < c ; i++) { char val = *tx++; - if (kp_spi_wait_for_reg_bit(cs, KP_SPI_REG_STATUS, KP_SPI_REG_STATUS_TXS) < 0) { + if (kp_spi_wait_for_reg_bit(cs, KP_SPI_REG_STATUS, + KP_SPI_REG_STATUS_TXS) + < 0) { goto out; } @@ -229,7 +260,9 @@ kp_spi_txrx_pio(struct spi_device *spidev, struct spi_transfer *transfer) kp_spi_write_reg(cs, KP_SPI_REG_TXDATA, 0x00); - if (kp_spi_wait_for_reg_bit(cs, KP_SPI_REG_STATUS, KP_SPI_REG_STATUS_RXS) < 0) { + if (kp_spi_wait_for_reg_bit(cs, KP_SPI_REG_STATUS, + KP_SPI_REG_STATUS_RXS) + < 0) { goto out; } @@ -239,8 +272,10 @@ kp_spi_txrx_pio(struct spi_device *spidev, struct spi_transfer *transfer) } } - if (kp_spi_wait_for_reg_bit(cs, KP_SPI_REG_STATUS, KP_SPI_REG_STATUS_EOT) < 0) { - //TODO: Figure out how to abort transaction?? This has never happened in practice though... + if (kp_spi_wait_for_reg_bit(cs, KP_SPI_REG_STATUS, + KP_SPI_REG_STATUS_EOT) < 0) { + //TODO: Figure out how to abort transaction?? + //Ths has never happened in practice though... } out: @@ -307,7 +342,8 @@ kp_spi_transfer_one_message(struct spi_master *master, struct spi_message *m) void *rx_buf = transfer->rx_buf; unsignedlen = transfer->len; -
[PATCH 0/8] staging: kpc2000: style refactoring
A patch-series that will remove warnings, errors and check-messages, noted and highlighted by the checkpatch.pl script concerning kpc2000_spi.c. Signed-off-by: Fabian Krueger Signed-off-by: Michael Scheiderer Cc: Fabian Krueger (8): staging: kpc2000: add line breaks staging: kpc2000: blank lines after declaration staging: kpc2000: introduce usage of __packed staging: kpc2000: add spaces staging: kpc2000: remove unnecessary brackets staging: kpc2000: introduce 'unsigned int' staging: kpc2000: introduce __func__ staging: kpc2000: remove needless 'break' drivers/staging/kpc2000/kpc2000_spi.c | 142 -- 1 file changed, 87 insertions(+), 55 deletions(-) -- 2.17.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 7/8] staging: kpc2000: introduce __func__
Instead of using the function name hard coded as string, using __func__ and the '%s'-placeholder will always give the current name of the function. When renaming a function, the debugging-messages won't have to be rewritten. Signed-off-by: Fabian Krueger Signed-off-by: Michael Scheiderer --- drivers/staging/kpc2000/kpc2000_spi.c | 8 +--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/drivers/staging/kpc2000/kpc2000_spi.c b/drivers/staging/kpc2000/kpc2000_spi.c index 732254e9b261..f258f369e065 100644 --- a/drivers/staging/kpc2000/kpc2000_spi.c +++ b/drivers/staging/kpc2000/kpc2000_spi.c @@ -457,13 +457,14 @@ kp_spi_probe(struct platform_device *pldev) drvdata = pldev->dev.platform_data; if (!drvdata) { - dev_err(>dev, "kp_spi_probe: platform_data is NULL!\n"); + dev_err(>dev, "%s: platform_data is NULL\n", __func__); return -ENODEV; } master = spi_alloc_master(>dev, sizeof(struct kp_spi)); if (master == NULL) { - dev_err(>dev, "kp_spi_probe: master allocation failed\n"); + dev_err(>dev, "%s: master allocation failed\n", + __func__); return -ENOMEM; } @@ -486,7 +487,8 @@ kp_spi_probe(struct platform_device *pldev) r = platform_get_resource(pldev, IORESOURCE_MEM, 0); if (r == NULL) { - dev_err(>dev, "kp_spi_probe: Unable to get platform resources\n"); + dev_err(>dev, "%s: Unable to get platform resources\n", + __func__); status = -ENODEV; goto free_master; } -- 2.17.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: comedi: use dma_mmap_coherent for DMA-able buffer mmap
Comedi's acquisition buffer allocation code can allocate the buffer from normal kernel memory or from DMA coherent memory depending on the `dma_async_dir` value in the comedi subdevice. (A value of `DMA_NONE` causes the buffer to be allocated from normal kernel memory. Other values cause the buffer to be allocated from DMA coherent memory.) The buffer currently consists of a bunch of page-sized blocks that are vmap'ed into a contiguous range of virtual addresses. The pages are also mmap'able into user address space. For DMA'able buffers, these page-sized blocks are allocated by `dma_alloc_coherent()`. For DMA-able buffers, the DMA API is currently abused in various ways, the most serious abuse being the calling of `virt_to_page()` on the blocks allocated by `dma_alloc_coherent()` and passing those pages to `vmap()` (for mapping to the kernels vmalloc address space) and via `page_to_pfn()` to `remap_pfn_range()` (for mmap'ing to user space). it also uses the `__GFP_COMP` flag when allocating the blocks, and marks the allocated pages as reserved (which is unnecessary for DMA coherent allocations). The code can be changed to get rid of the vmap'ed address altogether if necessary, since there are only a few places in the comedi code that use the vmap'ed address directly and we also keep a list of the kernel addresses for the individual pages prior to the vmap operation. This would add some run-time overhead to buffer accesses. The real killer is the mmap operation. For mmap, the address range specified in the VMA needs to be mmap'ed to the individually allocated page-sized blocks. That is not a problem when the pages are allocated from normal kernel memory as the individual pages can be remapped by `remap_pfn_range()`, but it is a problem when the page-sized blocks are allocated by `dma_alloc_coherent()` because the DMA API currently has no support for splitting a VMA across multiple blocks of DMA coherent memory (or rather, no support for mapping part of a VMA range to a single block of DMA coherent memory). In order to comply with the DMA API and allow the buffer to be mmap'ed, the buffer needs to be allocated as a single block by a single call to `dma_alloc_coherent()`, freed by a single call to `dma_free_coherent()`, and mmap'ed to user space by a single call to `dma_mmap_coherent()`. This patch changes the buffer allocation, freeing, and mmap'ing code to do that, with the unfortunate consequence that buffer allocation is more likely to fail. It also no longer uses the `__GFP_COMP` flag when allocating DMA coherent memory, no longer marks the allocated pages of DMA coherent memory as reserved, and no longer vmap's the DMA coherent memory pages (since they are contiguous anyway). Cc: Christoph Hellwig Signed-off-by: Ian Abbott --- drivers/staging/comedi/comedi_buf.c | 150 ++- drivers/staging/comedi/comedi_fops.c | 39 --- 2 files changed, 125 insertions(+), 64 deletions(-) diff --git a/drivers/staging/comedi/comedi_buf.c b/drivers/staging/comedi/comedi_buf.c index d2c8cc72a99d..3ef3ddabf139 100644 --- a/drivers/staging/comedi/comedi_buf.c +++ b/drivers/staging/comedi/comedi_buf.c @@ -27,18 +27,19 @@ static void comedi_buf_map_kref_release(struct kref *kref) unsigned int i; if (bm->page_list) { - for (i = 0; i < bm->n_pages; i++) { - buf = >page_list[i]; - clear_bit(PG_reserved, - &(virt_to_page(buf->virt_addr)->flags)); - if (bm->dma_dir != DMA_NONE) { -#ifdef CONFIG_HAS_DMA - dma_free_coherent(bm->dma_hw_dev, - PAGE_SIZE, - buf->virt_addr, - buf->dma_addr); -#endif - } else { + if (bm->dma_dir != DMA_NONE) { + /* +* DMA buffer was allocated as a single block. +* Address is in page_list[0]. +*/ + buf = >page_list[0]; + dma_free_coherent(bm->dma_hw_dev, + PAGE_SIZE * bm->n_pages, + buf->virt_addr, buf->dma_addr); + } else { + for (i = 0; i < bm->n_pages; i++) { + buf = >page_list[i]; + ClearPageReserved(virt_to_page(buf->virt_addr)); free_page((unsigned long)buf->virt_addr); } } @@ -57,7 +58,8 @@ static void __comedi_buf_free(struct comedi_device *dev, unsigned long flags; if (async->prealloc_buf) { - vunmap(async->prealloc_buf); + if (s->async_dma_dir == DMA_NONE) +
[PATCH] staging: media: davinci_vpfe: prefer using BIT macro
Use BIT(x) instead of (1< --- drivers/staging/media/davinci_vpfe/dm365_ipipe.h | 2 +- drivers/staging/media/davinci_vpfe/dm365_isif.h | 4 ++-- drivers/staging/media/davinci_vpfe/dm365_isif_regs.h | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/staging/media/davinci_vpfe/dm365_ipipe.h b/drivers/staging/media/davinci_vpfe/dm365_ipipe.h index 866ae12aeb07..de9b04683d82 100644 --- a/drivers/staging/media/davinci_vpfe/dm365_ipipe.h +++ b/drivers/staging/media/davinci_vpfe/dm365_ipipe.h @@ -99,7 +99,7 @@ struct ipipe_module_params { #define IPIPE_PADS_NUM 2 #define IPIPE_OUTPUT_NONE 0 -#define IPIPE_OUTPUT_RESIZER (1 << 0) +#define IPIPE_OUTPUT_RESIZER BIT(0) enum ipipe_input_entity { IPIPE_INPUT_NONE = 0, diff --git a/drivers/staging/media/davinci_vpfe/dm365_isif.h b/drivers/staging/media/davinci_vpfe/dm365_isif.h index 0e1fe472fb2b..288a1211b387 100644 --- a/drivers/staging/media/davinci_vpfe/dm365_isif.h +++ b/drivers/staging/media/davinci_vpfe/dm365_isif.h @@ -172,8 +172,8 @@ enum isif_input_entity { }; #define ISIF_OUTPUT_NONE (0) -#define ISIF_OUTPUT_MEMORY (1 << 0) -#define ISIF_OUTPUT_IPIPEIF(1 << 1) +#define ISIF_OUTPUT_MEMORY BIT(0) +#define ISIF_OUTPUT_IPIPEIFBIT(1) struct vpfe_isif_device { struct v4l2_subdev subdev; diff --git a/drivers/staging/media/davinci_vpfe/dm365_isif_regs.h b/drivers/staging/media/davinci_vpfe/dm365_isif_regs.h index 6695680817b9..e64b75dc1e5a 100644 --- a/drivers/staging/media/davinci_vpfe/dm365_isif_regs.h +++ b/drivers/staging/media/davinci_vpfe/dm365_isif_regs.h @@ -284,8 +284,8 @@ #define ISIF_LIN_ENTRY_MASK0x3ff /* masks and shifts*/ -#define ISIF_SYNCEN_VDHDEN_MASK(1 << 0) -#define ISIF_SYNCEN_WEN_MASK (1 << 1) +#define ISIF_SYNCEN_VDHDEN_MASKBIT(0) +#define ISIF_SYNCEN_WEN_MASK BIT(1) #define ISIF_SYNCEN_WEN_SHIFT 1 #endif /* _DAVINCI_VPFE_DM365_ISIF_REGS_H */ -- 2.22.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 3/4] staging: kpc2000: remove unnecessary parentheses in kpc2000_spi.c
Fixes checkpatch "CHECK: Unnecessary parentheses around '...'". Signed-off-by: Simon Sandström --- drivers/staging/kpc2000/kpc2000_spi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/kpc2000/kpc2000_spi.c b/drivers/staging/kpc2000/kpc2000_spi.c index 98484fbb9d2e..68b049f9ad69 100644 --- a/drivers/staging/kpc2000/kpc2000_spi.c +++ b/drivers/staging/kpc2000/kpc2000_spi.c @@ -164,7 +164,7 @@ kp_spi_read_reg(struct kp_spi_controller_state *cs, int idx) u64 val; addr += idx; - if ((idx == KP_SPI_REG_CONFIG) && (cs->conf_cache >= 0)) { + if (idx == KP_SPI_REG_CONFIG && cs->conf_cache >= 0) { return cs->conf_cache; } val = readq(addr); -- 2.20.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 4/4] staging: kpc2000: fix brace issues in kpc2000_spi.c
Fixes checkpatch errors: "else should follow close brace '}'" and "braces {} are not necessary for single statement blocks". Signed-off-by: Simon Sandström --- drivers/staging/kpc2000/kpc2000_spi.c | 10 -- 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/drivers/staging/kpc2000/kpc2000_spi.c b/drivers/staging/kpc2000/kpc2000_spi.c index 68b049f9ad69..4b1468137703 100644 --- a/drivers/staging/kpc2000/kpc2000_spi.c +++ b/drivers/staging/kpc2000/kpc2000_spi.c @@ -164,9 +164,9 @@ kp_spi_read_reg(struct kp_spi_controller_state *cs, int idx) u64 val; addr += idx; - if (idx == KP_SPI_REG_CONFIG && cs->conf_cache >= 0) { + if (idx == KP_SPI_REG_CONFIG && cs->conf_cache >= 0) return cs->conf_cache; - } + val = readq(addr); return val; } @@ -222,8 +222,7 @@ kp_spi_txrx_pio(struct spi_device *spidev, struct spi_transfer *transfer) kp_spi_write_reg(cs, KP_SPI_REG_TXDATA, val); processed++; } - } - else if (rx) { + } else if (rx) { for (i = 0 ; i < c ; i++) { char test = 0; @@ -261,9 +260,8 @@ kp_spi_setup(struct spi_device *spidev) cs = spidev->controller_state; if (!cs) { cs = kzalloc(sizeof(*cs), GFP_KERNEL); - if (!cs) { + if (!cs) return -ENOMEM; - } cs->base = kpspi->base; cs->conf_cache = -1; spidev->controller_state = cs; -- 2.20.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 0/4] Minor style issue fixes for staging/kpc2000
Hi, Here are some fixes for minor space, parenthese and brace issues in kpc2000 reported by checkpatch.pl. - Simon Simon Sandström (4): staging: kpc2000: add missing spaces in kpc2000_i2c.c staging: kpc2000: add missing spaces in kpc2000_spi.c staging: kpc2000: remove unnecessary parentheses in kpc2000_spi.c staging: kpc2000: fix brace issues in kpc2000_spi.c drivers/staging/kpc2000/kpc2000_i2c.c | 6 +++--- drivers/staging/kpc2000/kpc2000_spi.c | 18 -- 2 files changed, 11 insertions(+), 13 deletions(-) -- 2.20.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/4] staging: kpc2000: add missing spaces in kpc2000_spi.c
Fixes checkpatch errors: - spaces required around that '=' (ctx:VxV) - space required before the open parenthesis '(' - spaces preferred around that '-' (ctx:VxV) - space required before the open brace '{' Signed-off-by: Simon Sandström --- drivers/staging/kpc2000/kpc2000_spi.c | 14 +++--- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/drivers/staging/kpc2000/kpc2000_spi.c b/drivers/staging/kpc2000/kpc2000_spi.c index c3e5c1848f53..98484fbb9d2e 100644 --- a/drivers/staging/kpc2000/kpc2000_spi.c +++ b/drivers/staging/kpc2000/kpc2000_spi.c @@ -164,7 +164,7 @@ kp_spi_read_reg(struct kp_spi_controller_state *cs, int idx) u64 val; addr += idx; - if ((idx == KP_SPI_REG_CONFIG) && (cs->conf_cache >= 0)){ + if ((idx == KP_SPI_REG_CONFIG) && (cs->conf_cache >= 0)) { return cs->conf_cache; } val = readq(addr); @@ -223,9 +223,9 @@ kp_spi_txrx_pio(struct spi_device *spidev, struct spi_transfer *transfer) processed++; } } - else if(rx) { + else if (rx) { for (i = 0 ; i < c ; i++) { - char test=0; + char test = 0; kp_spi_write_reg(cs, KP_SPI_REG_TXDATA, 0x00); @@ -261,7 +261,7 @@ kp_spi_setup(struct spi_device *spidev) cs = spidev->controller_state; if (!cs) { cs = kzalloc(sizeof(*cs), GFP_KERNEL); - if(!cs) { + if (!cs) { return -ENOMEM; } cs->base = kpspi->base; @@ -364,7 +364,7 @@ kp_spi_transfer_one_message(struct spi_master *master, struct spi_message *m) if (transfer->bits_per_word) { word_len = transfer->bits_per_word; } - sc.bitfield.wl = word_len-1; + sc.bitfield.wl = word_len - 1; /* ...chip select */ sc.bitfield.cs = spidev->chip_select; @@ -425,7 +425,7 @@ kp_spi_probe(struct platform_device *pldev) int i; drvdata = pldev->dev.platform_data; - if (!drvdata){ + if (!drvdata) { dev_err(>dev, "kp_spi_probe: platform_data is NULL!\n"); return -ENODEV; } @@ -476,7 +476,7 @@ kp_spi_probe(struct platform_device *pldev) spi_new_device(master, &(table[i])); \ } - switch ((drvdata->card_id & 0x) >> 16){ + switch ((drvdata->card_id & 0x) >> 16) { case PCI_DEVICE_ID_DAKTRONICS_KADOKA_P2KR0: NEW_SPI_DEVICE_FROM_BOARD_INFO_TABLE(p2kr0_board_info); break; -- 2.20.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/4] staging: kpc2000: add missing spaces in kpc2000_i2c.c
Fixes checkpatch "CHECK: spaces preferred around that '+' (ctx:VxV)". Signed-off-by: Simon Sandström --- drivers/staging/kpc2000/kpc2000_i2c.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/staging/kpc2000/kpc2000_i2c.c b/drivers/staging/kpc2000/kpc2000_i2c.c index 69e8773c1ef8..3e08df9f205d 100644 --- a/drivers/staging/kpc2000/kpc2000_i2c.c +++ b/drivers/staging/kpc2000/kpc2000_i2c.c @@ -257,7 +257,7 @@ static int i801_block_transaction_by_block(struct i2c_device *priv, union i2c_sm len = data->block[0]; outb_p(len, SMBHSTDAT0(priv)); for (i = 0; i < len; i++) - outb_p(data->block[i+1], SMBBLKDAT(priv)); + outb_p(data->block[i + 1], SMBBLKDAT(priv)); } status = i801_transaction(priv, I801_BLOCK_DATA | ENABLE_INT9 | I801_PEC_EN * hwpec); @@ -337,8 +337,8 @@ static int i801_block_transaction_byte_by_byte(struct i2c_device *priv, union i2 /* Retrieve/store value in SMBBLKDAT */ if (read_write == I2C_SMBUS_READ) data->block[i] = inb_p(SMBBLKDAT(priv)); - if (read_write == I2C_SMBUS_WRITE && i+1 <= len) - outb_p(data->block[i+1], SMBBLKDAT(priv)); + if (read_write == I2C_SMBUS_WRITE && i + 1 <= len) + outb_p(data->block[i + 1], SMBBLKDAT(priv)); /* signals SMBBLKDAT ready */ outb_p(SMBHSTSTS_BYTE_DONE | SMBHSTSTS_INTR, SMBHSTSTS(priv)); } -- 2.20.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: bcm2835-camera: Restore return behavior of ctrl_set_bitrate()
On Tue, Jun 25, 2019 at 12:13:15AM +0200, Stefan Wahren wrote: > The commit 52c4dfcead49 ("Staging: vc04_services: Cleanup in > ctrl_set_bitrate()") changed the return behavior of ctrl_set_bitrate(). > This breaks probing of bcm2835-camera: > > bcm2835-v4l2: mmal_init: failed to set all camera controls: -3 > Cleanup: Destroy video encoder > Cleanup: Destroy image encoder > Cleanup: Destroy video render > Cleanup: Destroy camera > bcm2835-v4l2: bcm2835_mmal_probe: mmal init failed: -3 > bcm2835-camera: probe of bcm2835-camera failed with error -3 > > So restore the old behavior and fix this issue. > > Fixes: 52c4dfcead49 ("Staging: vc04_services: Cleanup in ctrl_set_bitrate()") > Signed-off-by: Stefan Wahren I feel like this papers over the issue. It would be better to figure out why this is failing and fix it properly. -3 is -ESRCH and when I grep for ESRCH I only see it used in the ioctl so that can't be it. I think it must be -MMAL_MSG_STATUS_EINVAL actually, but it comes from the firmware or something so we can't grep for it. Can we do some more digging to find out why it's failing or otherwise we could add a comment. /* * FIXME: port_parameter_set() sometimes fails with * -MMAL_MSG_STATUS_EINVAL and we don't know why so we're * ignoring those errors for now. * */ return 0; regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: bcm2835-camera: Restore return behavior of ctrl_set_bitrate()
On Mon, Jun 24, 2019 at 11:13 PM Stefan Wahren wrote: > > The commit 52c4dfcead49 ("Staging: vc04_services: Cleanup in > ctrl_set_bitrate()") changed the return behavior of ctrl_set_bitrate(). > This breaks probing of bcm2835-camera: > > bcm2835-v4l2: mmal_init: failed to set all camera controls: -3 > Cleanup: Destroy video encoder > Cleanup: Destroy image encoder > Cleanup: Destroy video render > Cleanup: Destroy camera > bcm2835-v4l2: bcm2835_mmal_probe: mmal init failed: -3 > bcm2835-camera: probe of bcm2835-camera failed with error -3 > > So restore the old behavior and fix this issue. > > Fixes: 52c4dfcead49 ("Staging: vc04_services: Cleanup in ctrl_set_bitrate()") > Signed-off-by: Stefan Wahren Tested-by: Peter Robinson Thanks Stefan, I can confirm this resolves the issue I have seen with the camera on 5.2 but hadn't had the time to bisect it yet. Tested with a v2.1 camera module attached to a RPI3A+ Regards, Peter > --- > drivers/staging/vc04_services/bcm2835-camera/controls.c | 8 +--- > 1 file changed, 5 insertions(+), 3 deletions(-) > > diff --git a/drivers/staging/vc04_services/bcm2835-camera/controls.c > b/drivers/staging/vc04_services/bcm2835-camera/controls.c > index d60e378..1c4c9e8 100644 > --- a/drivers/staging/vc04_services/bcm2835-camera/controls.c > +++ b/drivers/staging/vc04_services/bcm2835-camera/controls.c > @@ -610,9 +610,11 @@ static int ctrl_set_bitrate(struct bm2835_mmal_dev *dev, > > encoder_out = >component[MMAL_COMPONENT_VIDEO_ENCODE]->output[0]; > > - return vchiq_mmal_port_parameter_set(dev->instance, encoder_out, > -mmal_ctrl->mmal_id, >val, > -sizeof(ctrl->val)); > + vchiq_mmal_port_parameter_set(dev->instance, encoder_out, > + mmal_ctrl->mmal_id, >val, > + sizeof(ctrl->val)); > + > + return 0; > } > > static int ctrl_set_bitrate_mode(struct bm2835_mmal_dev *dev, > -- > 2.7.4 > > > ___ > linux-rpi-kernel mailing list > linux-rpi-ker...@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-rpi-kernel ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/1] staging: media: fix style problem
On 6/21/19 8:39 AM, Aliasgar Surti wrote: > From: Aliasgar Surti > > checkpatch reported "WARNING: line over 80 characters". > This patch fixes the warning for file soc_camera/soc_ov5642.c FYI: we're not accepting patches for staging/media/soc_camera: these are obsolete and broken drivers. Regards, Hans > > Signed-off-by: Aliasgar Surti > --- > drivers/staging/media/soc_camera/soc_ov5642.c | 6 -- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/drivers/staging/media/soc_camera/soc_ov5642.c > b/drivers/staging/media/soc_camera/soc_ov5642.c > index 94696d7..39ae24dc 100644 > --- a/drivers/staging/media/soc_camera/soc_ov5642.c > +++ b/drivers/staging/media/soc_camera/soc_ov5642.c > @@ -687,7 +687,8 @@ static int reg_write16(struct i2c_client *client, u16 > reg, u16 val16) > } > > #ifdef CONFIG_VIDEO_ADV_DEBUG > -static int ov5642_get_register(struct v4l2_subdev *sd, struct > v4l2_dbg_register *reg) > +static int ov5642_get_register(struct v4l2_subdev *sd, > +struct v4l2_dbg_register *reg) > { > struct i2c_client *client = v4l2_get_subdevdata(sd); > int ret; > @@ -705,7 +706,8 @@ static int ov5642_get_register(struct v4l2_subdev *sd, > struct v4l2_dbg_register > return ret; > } > > -static int ov5642_set_register(struct v4l2_subdev *sd, const struct > v4l2_dbg_register *reg) > +static int ov5642_set_register(struct v4l2_subdev *sd, > +const struct v4l2_dbg_register *reg) > { > struct i2c_client *client = v4l2_get_subdevdata(sd); > > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v4 4/7] lib/hexdump.c: Replace ascii bool in hex_dump_to_buffer with flags
On Tue, 2019-06-25 at 13:17 +1000, Alastair D'Silva wrote: > From: Alastair D'Silva > > In order to support additional features, rename hex_dump_to_buffer to > hex_dump_to_buffer_ext, and replace the ascii bool parameter with flags. [] > diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c > b/drivers/gpu/drm/i915/intel_engine_cs.c [] > @@ -1338,9 +1338,8 @@ static void hexdump(struct drm_printer *m, const void > *buf, size_t len) > } > > WARN_ON_ONCE(hex_dump_to_buffer(buf + pos, len - pos, > - rowsize, sizeof(u32), > - line, sizeof(line), > - false) >= sizeof(line)); > + rowsize, sizeof(u32), line, > + sizeof(line)) >= sizeof(line)); Huh? Why do this? > diff --git a/drivers/isdn/hardware/mISDN/mISDNisar.c > b/drivers/isdn/hardware/mISDN/mISDNisar.c [] > @@ -70,8 +70,9 @@ send_mbox(struct isar_hw *isar, u8 his, u8 creg, u8 len, u8 > *msg) > int l = 0; > > while (l < (int)len) { > - hex_dump_to_buffer(msg + l, len - l, 32, 1, > -isar->log, 256, 1); > + hex_dump_to_buffer_ext(msg + l, len - l, 32, 1, > +isar->log, 256, > +HEXDUMP_ASCII); Again, why do any of these? The point of the wrapper is to avoid changing these. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v4 5/7] lib/hexdump.c: Allow multiple groups to be separated by lines '|'
On Tue, 2019-06-25 at 13:17 +1000, Alastair D'Silva wrote: > From: Alastair D'Silva > > With the wider display format, it can become hard to identify how many > bytes into the line you are looking at. > > The patch adds new flags to hex_dump_to_buffer() and print_hex_dump() to > print vertical lines to separate every N groups of bytes. > > eg. > buf:: 454d414e 43415053|4e495f45 00584544 NAMESPAC|E_INDEX. > buf:0010: 0002| | > > Signed-off-by: Alastair D'Silva > --- > include/linux/printk.h | 3 +++ > lib/hexdump.c | 59 -- > 2 files changed, 54 insertions(+), 8 deletions(-) > > diff --git a/include/linux/printk.h b/include/linux/printk.h [] > @@ -485,6 +485,9 @@ enum { > > #define HEXDUMP_ASCIIBIT(0) > #define HEXDUMP_SUPPRESS_REPEATEDBIT(1) > +#define HEXDUMP_2_GRP_LINES BIT(2) > +#define HEXDUMP_4_GRP_LINES BIT(3) > +#define HEXDUMP_8_GRP_LINES BIT(4) These aren't really bits as only one value should be set as 8 overrides 4 and 4 overrides 2. I would also expect this to be a value of 2 in your above example, rather than 8. It's described as groups not bytes. The example is showing a what would normally be a space ' ' separator as a vertical bar '|' every 2nd grouping. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel