From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753946AbcGUBJb (ORCPT ); Wed, 20 Jul 2016 21:09:31 -0400 Received: from mailout3.samsung.com ([203.254.224.33]:36263 "EHLO mailout3.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753471AbcGUBJ3 (ORCPT ); Wed, 20 Jul 2016 21:09:29 -0400 X-AuditID: cbfee68d-f79876d000001436-0b-579020c6e2c1 Date: Thu, 21 Jul 2016 10:09:26 +0900 From: Andi Shyti To: Sean Young Cc: Mauro Carvalho Chehab , linux-media@vger.kernel.org, linux-kernel@vger.kernel.org, Andi Shyti Subject: Re: [RFC 7/7] [media] rc: add support for IR LEDs driven through SPI Message-id: <20160721010926.GG23521@samsunx.samsung> References: <1468943818-26025-1-git-send-email-andi.shyti@samsung.com> <1468943818-26025-8-git-send-email-andi.shyti@samsung.com> <20160719231122.GA25146@gofer.mess.org> MIME-version: 1.0 Content-type: text/plain; charset=us-ascii Content-disposition: inline In-reply-to: <20160719231122.GA25146@gofer.mess.org> User-Agent: Mutt/1.6.0 (2016-04-01) X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFprCIsWRmVeSWpSXmKPExsWyRsSkQPeYwoRwg54JFhaLfzxnsri8aw6b Rc+GrawWq59VWCx8+p3FgdXj+pJPzB5L3lh7bOm/y+7xeZNcAEsUl01Kak5mWWqRvl0CV8ak hh2sBQc5Ki69WsrawHiVrYuRk0NCwETi1tx3jBC2mMSFe+uB4lwcQgIrGCU2Xz3MAlM0+fwm dojEUkaJPfOWQDkfGSX+NTxkBaliEVCVWNd+CcxmE9CUaLr9A2yFiICcxLdtLawgDcwCExkl HvV3go0VFvCVuH/qBNhuXqAVpydMY4SY2sgksbXtBDtEQlDix+R7YA3MAloS63ceZ4KwpSUe /Z0BVsMJ1DxrzV6gZg4OUQFlibvLgkDmSAhsY5fYv2k3I8R1AhLfJh9iAamREJCV2HSAGeI1 SYmDK26wTGAUm4Vk2ywk22Yh2baAkXkVo2hqQXJBcVJ6kaFecWJucWleul5yfu4mRmBknf73 rHcH4+0D1ocYBTgYlXh4V7zoDxdiTSwrrsw9xGgKdMVEZinR5Hxg/OaVxBsamxlZmJqYGhuZ W5opifMqSv0MFhJITyxJzU5NLUgtii8qzUktPsTIxMEp1cC469f8gMWrGENzOlP1blbEhx2+ kr6leP0P/7xrtVmMD3R4l9Y3h6Vf+qzl91k7udkx+KrFST43G6vjRT53N2szLj7zTXaebJmB 1mWV4xKS1n3H+9YHnJq6yLXRqP3jgfLqmaUam7XPmpX55Bmbd1yqufXl1+H59+unHOP8suvo nGOmnk/Nfv1VYinOSDTUYi4qTgQA2SUQyKcCAAA= X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFnrIIsWRmVeSWpSXmKPExsVy+t9jAd1jChPCDR5PMLZY/OM5k8XlXXPY LHo2bGW1WP2swmLh0+8sDqwe15d8YvZY8sbaY0v/XXaPz5vkAliiGhhtMlITU1KLFFLzkvNT MvPSbZW8g+Od403NDAx1DS0tzJUU8hJzU22VXHwCdN0yc4DWKimUJeaUAoUCEouLlfTtME0I DXHTtYBpjND1DQmC6zEyQAMJaxgzJjXsYC04yFFx6dVS1gbGq2xdjJwcEgImEpPPb2KHsMUk LtxbDxTn4hASWMoosWfeEnYI5yOjxL+Gh6wgVSwCqhLr2i+B2WwCmhJNt3+ATRIRkJP4tq2F FaSBWWAio8Sj/k4WkISwgK/E/VMnGEFsXqB1pydMY4SY2sgksbXtBDtEQlDix+R7YA3MAloS 63ceZ4KwpSUe/Z0BVsMJ1DxrzV6gZg4OUQFlibvLgiYwCsxC0j0LSfcsJN0LGJlXMUqkFiQX FCel5xrlpZbrFSfmFpfmpesl5+duYgRH7zPpHYyHd7kfYhTgYFTi4d2xuj9ciDWxrLgy9xCj BAezkgivjsiEcCHelMTKqtSi/Pii0pzU4kOMpsAQmcgsJZqcD0wseSXxhsYmZkaWRuaGFkbG 5krivI//rwsTEkhPLEnNTk0tSC2C6WPi4JRqYJxbpRiVufC02wPdydrHDeq2O335tztWqrh1 wsWOrDLFN+VL/CvyW5z3JOaW2d6cyPdV/uqMgPZ8HjX2ZdFtLprzVoloVide4/p8sCf7uUCJ LIvUMeck9R97VEJWsJydys/t+mPzpaOSaZGyHkFlR9/+rPjZUC63PvTA26TijYIFPw+bCE35 p8RSnJFoqMVcVJwIAI/cp8P0AgAA DLP-Filter: Pass X-MTR: 20000000000000000@CPGS X-CFilter-Loop: Reflected Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Sean, > > + int ret; > > + struct ir_spi_data *idata = (struct ir_spi_data *) dev->priv; > > No cast needed. yes, thanks. > > + ret = regulator_enable(idata->regulator); > > + if (ret) > > + return ret; > > + > > + mutex_lock(&idata->mutex); > > + idata->xfer.len = n; > > + idata->xfer.tx_buf = buffer; > > + mutex_unlock(&idata->mutex); > > I'm not convinced the locking works here. You want to guard against > someone modifying xfer while you are sending (so in spi_sync_transfer), > which this locking is not doing. You could declare a > local "struct spi_transfer xfer" and avoid the mutex altogether. I cannot declare xfer locally because the spi framework needs a statically allocated xfer, so that either I dynamically allocate it in the function or I declare it global in idata. With the mutex I would like to prevent different tasks to change the value at the same time, it's an easy case, it shouldn't make much difference. There are checkpatch issues, in the next patchset I will fix them. Thanks a lot for your review, Andi