From mboxrd@z Thu Jan 1 00:00:00 1970 From: zhangfei Subject: Re: [PATCH v2] i2c: designware: add reset interface Date: Fri, 16 Dec 2016 11:01:29 +0800 Message-ID: <49a5d8a4-864e-e80e-eee0-57c876d25aaf@linaro.org> References: <1479789700-19532-1-git-send-email-zhangfei.gao@linaro.org> <1481792388-13781-1-git-send-email-zhangfei.gao@linaro.org> <1481805227.9552.15.camel@linux.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Return-path: Received: from mail-pg0-f45.google.com ([74.125.83.45]:33508 "EHLO mail-pg0-f45.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751710AbcLPDBj (ORCPT ); Thu, 15 Dec 2016 22:01:39 -0500 Received: by mail-pg0-f45.google.com with SMTP id 3so26798823pgd.0 for ; Thu, 15 Dec 2016 19:01:39 -0800 (PST) In-Reply-To: Sender: linux-i2c-owner@vger.kernel.org List-Id: linux-i2c@vger.kernel.org To: Ramiro Oliveira , Andy Shevchenko , Wolfram Sang , "mika.westerberg@linux.intel.com" , "jarkko.nikula@linux.intel.com" , Philipp Zabel Cc: "linux-arm-kernel@lists.infradead.org" , "linux-i2c@vger.kernel.org" Hi, Philipp On 2016年12月16日 10:45, zhangfei wrote: > > > On 2016年12月15日 23:30, Ramiro Oliveira wrote: >> Hi Andy and Zhangfei >> >> On 12/15/2016 12:33 PM, Andy Shevchenko wrote: >>> On Thu, 2016-12-15 at 16:59 +0800, Zhangfei Gao wrote: >>>> Some platforms like hi3660 need do reset first to allow accessing >>>> registers >>> Patch itself looks good, but would be nice to have it tested. >>> >>> Reviewed-by: Andy Shevchenko >>> >> I tested the patch and it's working for the ARC architecture. >> >>>> Signed-off-by: Zhangfei Gao >>>> --- >>>> drivers/i2c/busses/i2c-designware-core.h | 1 + >>>> drivers/i2c/busses/i2c-designware-platdrv.c | 28 >>>> ++++++++++++++++++++++++---- >>>> 2 files changed, 25 insertions(+), 4 deletions(-) >>>> >>>> diff --git a/drivers/i2c/busses/i2c-designware-core.h >>>> b/drivers/i2c/busses/i2c-designware-core.h >>>> index 0d44d2a..94b14fa 100644 >>>> --- a/drivers/i2c/busses/i2c-designware-core.h >>>> +++ b/drivers/i2c/busses/i2c-designware-core.h >>>> @@ -80,6 +80,7 @@ struct dw_i2c_dev { >>>> void __iomem *base; >>>> struct completion cmd_complete; >>>> struct clk *clk; >>>> + struct reset_control *rst; >>>> u32 (*get_clk_rate_khz) (struct >>>> dw_i2c_dev *dev); >>>> struct dw_pci_controller *controller; >>>> int cmd_err; >>>> diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c >>>> b/drivers/i2c/busses/i2c-designware-platdrv.c >>>> index 0b42a12..e9016ae 100644 >>>> --- a/drivers/i2c/busses/i2c-designware-platdrv.c >>>> +++ b/drivers/i2c/busses/i2c-designware-platdrv.c >>>> @@ -38,6 +38,7 @@ >>>> #include >>>> #include >>>> #include >>>> +#include >>>> #include >>>> #include >>>> #include >>>> @@ -176,6 +177,14 @@ static int dw_i2c_plat_probe(struct >>>> platform_device *pdev) >>>> dev->irq = irq; >>>> platform_set_drvdata(pdev, dev); >>>> + dev->rst = devm_reset_control_get_optional(&pdev->dev, NULL); >> devm_reset_control_get_optional() is deprecated as explained in >> linux/reset.h, >> you should use devm_reset_control_get_optional_exclusive() or >> devm_reset_control_get_optional_shared() instead, as applicable. >> >> I submitted a similar patch earlier today and I made the same mistake. > > Thanks Ramiro for the info > Will use devm_reset_control_get_optional_exclusive instead. > But should the interface be as simple as possible? > > Thanks Sorry, a bit confused. Is that mean we should not use devm_reset_control_get_optional() While driver should decide whether use devm_reset_control_get_optional_exclusive() or devm_reset_control_get_optional_shared() What if different platform has different requirement? Looks the difference between _exclusive and _shared is refcount, How about handle this inside, and not decided by interface? Thanks