From mboxrd@z Thu Jan 1 00:00:00 1970 From: Suravee Suthikulanit Subject: Re: [PATCH v2] i2c: designware: Do not require clock when SSCN and FFCN are provided Date: Tue, 22 Dec 2015 14:51:13 -0600 Message-ID: <5679B7C1.6010408@amd.com> References: <1450319025-19120-1-git-send-email-Suravee.Suthikulpanit@amd.com> <20151218101348.GG1762@lahna.fi.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20151218101348.GG1762@lahna.fi.intel.com> Sender: linux-i2c-owner@vger.kernel.org To: Mika Westerberg Cc: wsa@the-dreams.de, jarkko.nikula@linux.intel.com, andriy.shevchenko@linux.intel.com, lho@apm.com, Ken.Xue@amd.com, linux-i2c@vger.kernel.org, linux-acpi@vger.kernel.org, linux-kernel@vger.kernel.org List-Id: linux-acpi@vger.kernel.org Hi Mika, On 12/18/2015 4:13 AM, Mika Westerberg wrote: > [....] > So instead of this, what if we do not assign dev->get_clk_rate_khz at > all and then do something like below in the core driver? I like the changes below since it is clear to see within the core file how things are handled when get_clk_rate_khz is not assigned (i.e. input_clock_hz = 0), and not necessary relying on the platform driver to return 0 in this case. So, at this point, I can re-submit the V3 and combine these changes, and we both can sign-off. How does that sound? Thanks, Suravee > Of course we still need the other changes you did in this patch to cope > with the missing clock. > > diff --git a/drivers/i2c/busses/i2c-designware-core.c b/drivers/i2c/busses/i2c-designware-core.c > index 8c48b27ba059..25dccd8df772 100644 > --- a/drivers/i2c/busses/i2c-designware-core.c > +++ b/drivers/i2c/busses/i2c-designware-core.c > @@ -271,6 +271,17 @@ static void __i2c_dw_enable(struct dw_i2c_dev *dev, bool enable) > enable ? "en" : "dis"); > } > > +static unsigned long i2c_dw_clk_rate(struct dw_i2c_dev *dev) > +{ > + /* > + * Clock is not necessary if we got LCNT/HCNT values directly from > + * the platform code. > + */ > + if (WARN_ON_ONCE(!dev->get_clk_rate_khz)) > + return 0; > + return dev->get_clk_rate_khz(dev); > +} > + > /** > * i2c_dw_init() - initialize the designware i2c master hardware > * @dev: device private data > @@ -281,7 +292,6 @@ static void __i2c_dw_enable(struct dw_i2c_dev *dev, bool enable) > */ > int i2c_dw_init(struct dw_i2c_dev *dev) > { > - u32 input_clock_khz; > u32 hcnt, lcnt; > u32 reg; > u32 sda_falling_time, scl_falling_time; > @@ -295,8 +305,6 @@ int i2c_dw_init(struct dw_i2c_dev *dev) > } > } > > - input_clock_khz = dev->get_clk_rate_khz(dev); > - > reg = dw_readl(dev, DW_IC_COMP_TYPE); > if (reg == ___constant_swab32(DW_IC_COMP_TYPE_VALUE)) { > /* Configure register endianess access */ > @@ -325,12 +333,12 @@ int i2c_dw_init(struct dw_i2c_dev *dev) > hcnt = dev->ss_hcnt; > lcnt = dev->ss_lcnt; > } else { > - hcnt = i2c_dw_scl_hcnt(input_clock_khz, > + hcnt = i2c_dw_scl_hcnt(i2c_dw_clk_rate(dev), > 4000, /* tHD;STA = tHIGH = 4.0 us */ > sda_falling_time, > 0, /* 0: DW default, 1: Ideal */ > 0); /* No offset */ > - lcnt = i2c_dw_scl_lcnt(input_clock_khz, > + lcnt = i2c_dw_scl_lcnt(i2c_dw_clk_rate(dev), > 4700, /* tLOW = 4.7 us */ > scl_falling_time, > 0); /* No offset */ > @@ -344,12 +352,12 @@ int i2c_dw_init(struct dw_i2c_dev *dev) > hcnt = dev->fs_hcnt; > lcnt = dev->fs_lcnt; > } else { > - hcnt = i2c_dw_scl_hcnt(input_clock_khz, > + hcnt = i2c_dw_scl_hcnt(i2c_dw_clk_rate(dev), > 600, /* tHD;STA = tHIGH = 0.6 us */ > sda_falling_time, > 0, /* 0: DW default, 1: Ideal */ > 0); /* No offset */ > - lcnt = i2c_dw_scl_lcnt(input_clock_khz, > + lcnt = i2c_dw_scl_lcnt(i2c_dw_clk_rate(dev), > 1300, /* tLOW = 1.3 us */ > scl_falling_time, > 0); /* No offset */ > From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933768AbbLVUvZ (ORCPT ); Tue, 22 Dec 2015 15:51:25 -0500 Received: from mail-by2on0067.outbound.protection.outlook.com ([207.46.100.67]:36891 "EHLO na01-by2-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S933323AbbLVUvW (ORCPT ); Tue, 22 Dec 2015 15:51:22 -0500 Authentication-Results: spf=none (sender IP is ) smtp.mailfrom=Suravee.Suthikulpanit@amd.com; Subject: Re: [PATCH v2] i2c: designware: Do not require clock when SSCN and FFCN are provided To: Mika Westerberg References: <1450319025-19120-1-git-send-email-Suravee.Suthikulpanit@amd.com> <20151218101348.GG1762@lahna.fi.intel.com> CC: , , , , , , , From: Suravee Suthikulanit Message-ID: <5679B7C1.6010408@amd.com> Date: Tue, 22 Dec 2015 14:51:13 -0600 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:38.0) Gecko/20100101 Thunderbird/38.2.0 MIME-Version: 1.0 In-Reply-To: <20151218101348.GG1762@lahna.fi.intel.com> Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit X-Originating-IP: [165.204.77.1] X-ClientProxiedBy: SN2PR10CA0036.namprd10.prod.outlook.com (25.160.12.174) To BY1PR12MB0439.namprd12.prod.outlook.com (25.162.147.140) X-Microsoft-Exchange-Diagnostics: 1;BY1PR12MB0439;2:+qixpRAUkTu5T/iEB9FsdKI0Ft5dkRpYDVFGf20VtULo45zxbCvhUtPIHW5jT6GJF+ScuuXkl1z//4ehacsNo8EifkKsQRqHIZqD2c8Y8JE6eJECnMjXM1Esnrcg5esQ9SxJY02NW/zOvE9SLfNYiQ==;3:Dj8/7EzqbWJTdBAsNzfWypKr5DR00OMjO/QMzxo8pUUPo8wnDhwvqvC3weJPUTr3GMU2jbY+HO+deUP9QLf2DXos2mX7ZSb2YSBEPqYTc0IM/fZR7eN3W2i/XzKiZPB+;25:OGorkDgJRqVL2foEBjUJs5JisonVBNyqCxxU1I/pm9qbKWIjYjzb/gLfGC+i03AKEYDk+sWyJdB/YSFTy6lTO6ASy8I0nUTM+lJ2yts4sG2k3ybZVLheohSYMZEMJc4iHTTZaHhBKGosKYHpcZ+sKR3BTc0rwYL8yRoCJjT6Mk8Khjc2VtQGRGjT5TaUPcvEfmI0jOUtCgBdK7vsMm/upnwoXWBLiWyxgdSIbC8OsrevLJ+woFDSCopZCj2LoLwzQ8wB0CaidUgzccz4DwqA+Q== X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:BY1PR12MB0439; X-Microsoft-Exchange-Diagnostics: 1;BY1PR12MB0439;20:E2Xaf9eeMHsIQJBzNVpMqwa6sCp01IUg4R4r7J2mhh9DyKBEfmrOcqow9FnWz8/II8AVPGhIBBAY89yU13ExbaX2RemCOtsEVUeAKtO2vg7xGLsO8w57VYvLZp0druZkas3w4kf6JJJmyOUlXxea9lbnekN/p3z2MSFBlZEgSvIr3izW64zgzrFm05vBM/BYxdMp+axwmG2xCIp26KkPBo0dPR+Pp+bMhUjD+MU5MK6OqPJBkBa6mzfTjdzphzyvwqGhHOW4TNlcIJeIr7Q1qxKDq24k63pKSaAPZdxr5guuXxg41vdqnSmmtB4qXUUTtk6oO3PiOxDxL8kl6SrKrX34Lm5yOg+4t9mjNsNf0vivwhXF/XUniE++E+6lAsWANI2B5TQr3gzDhMEoei9rtrmNmt0YxFM8C97hG7K1OfjWtNBAgRZjJiLZbYpJhxNZngmmn+wcDW74lsoWQ54Aed75VCg4sX+D5jPB57Nwti8S9HEvE7b3zee/m3kwwOZu;4:OeOL+14AEzvN2O/4fkqEsMckp6h2t4fqnyS0ZdAETzdu4t5OxEW+x4vKgEMGWg+GrP6huD01YlKLvZjhM2ud5NDjWg+c4ctVM+JVjopB8eTFgR2IQ8C5YFQgQvFfvv0CFCht1jTZcPx+tPhGUq4JPYv9ZmQLsGqfQmakLLXgRI1UmfOh7wM8ngHg41Bp793jkZx/lG6DJDXDPbqpnmWFTBH90bVzRdMjmcmxMv8QltjLpWLXK2dwt9kGkjbFsVbGt0BRuIExrSyuE9GF+LAOJlK5P815EFOh5GqEiArGQfeBrhy48BircXpNeu3eJfeXMlSYB3kZ1k45trqjQ1AfO70foa7NxfExzZNCSdZo0VMUuBP09su1un1B8iqL64v8 X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:; X-Exchange-Antispam-Report-CFA-Test: BCL:0;PCL:0;RULEID:(601004)(2401047)(520078)(5005006)(8121501046)(3002001)(10201501046);SRVR:BY1PR12MB0439;BCL:0;PCL:0;RULEID:;SRVR:BY1PR12MB0439; X-Forefront-PRVS: 0798146F16 X-Forefront-Antispam-Report: SFV:NSPM;SFS:(10009020)(6009001)(6049001)(377454003)(199003)(189002)(164054003)(479174004)(24454002)(50466002)(23746002)(81156007)(122386002)(4001350100001)(54356999)(105586002)(80316001)(40100003)(87266999)(189998001)(5001960100002)(87976001)(42186005)(230700001)(86362001)(33656002)(106356001)(59896002)(97736004)(586003)(2950100001)(5008740100001)(6116002)(3846002)(83506001)(65806001)(1096002)(64126003)(47776003)(65956001)(66066001)(110136002)(50986999)(65816999)(92566002)(101416001)(76176999)(5004730100002)(36756003)(77096005);DIR:OUT;SFP:1101;SCL:1;SRVR:BY1PR12MB0439;H:[10.236.18.83];FPR:;SPF:None;PTR:InfoNoRecords;A:1;MX:1;LANG:en; X-Microsoft-Exchange-Diagnostics: =?Windows-1252?Q?1;BY1PR12MB0439;23:iZV6zoQPlsIq6vekVWeCSuwcaIlCWGd0tqUYp?= =?Windows-1252?Q?QLgMGGLQmE+FZ50GQNC3EfzijFonG3RtA6UXtRiCq9wJORQXjwyktDUH?= =?Windows-1252?Q?OzJU7v7IiqNqh/KPayAaCxCZ+ym8Jla6VF1BR6EoK61duatNh39Eks+V?= =?Windows-1252?Q?QrS4xXc41bscQzmLnezTX7TKEsrToQTaFkVdAXUFIESXtp1qxh5x4Kzb?= =?Windows-1252?Q?9tyWZhHbO+i/fDScofpPg+/fiWOHqdwRliYbXAJwN4AIal89v2jCCZaG?= =?Windows-1252?Q?59q94e+icqcU1dX6fdF3Xn6KZ5/P7cPAxJaAe/CyQ0RP+oiI/P9xkrsp?= =?Windows-1252?Q?sy5ostdjCNcF/teiS6MseIBOAGOdAg4EFBJ2jz0aOZY31/PrSYbbRjAp?= =?Windows-1252?Q?XdahfmI8LxUPfGaKGm8IO9PCzt7cFMmwJD9FoBaB8T5UkKkfveOu8gcc?= =?Windows-1252?Q?eaJ4MYWvGqVmroDGL3uG/76uN2qV2RIWBb6n7V+TZKhnINk1m8RROzCL?= =?Windows-1252?Q?jJAGQ3l22HF0gRWSu6GPO7xpcee0ehfgaWjMJJCeZZUwpiEt2ZIrxwqe?= =?Windows-1252?Q?ZdpSJYEYWo3QZDtrp0IPUHW2sc4bhWX56HbhgHiMwjn9Ob5sXMR4LAeq?= =?Windows-1252?Q?jnbBEcs0Y1q1FleYn/Z97Wpn8xWmHCXPaDIWnufnn8cbnNgj3h7dMd6+?= =?Windows-1252?Q?AaCI/aSS8c/XfBHEyYLH96ffUSW3gcNoofNCOGYwanUY7LHrhcDLILNQ?= =?Windows-1252?Q?WtjQn6ThdvfVZgZmpXkXOi9a7HlLroL9vPoHEHex0QAxoV7fY5D/ZJQd?= =?Windows-1252?Q?UyIlKCtx12p74M+P4zH23Wv6nMAjBCrUTfP63kuhbkXctmErFl8HQp+n?= =?Windows-1252?Q?2Yn+DsoC5l+K2RtDxFmcayTkT8ihFB3lS9NKJyO2fzLUoJccgwnJobqU?= =?Windows-1252?Q?CXCfr/hpbjHhUc14wTWY2I4qrqiB7VzsR3ZcsmUXQajIA6yBoZDNSmP0?= =?Windows-1252?Q?ny+1CRjl0A+fOzW6CiFrVyEiVuo+HAa0Tehyu7ZhZNZMC2CWgaVRH1RR?= =?Windows-1252?Q?LS+dBupUg7cRd2MFTM6yX5LUr5c7tqYpuTKdsMBm+3XyYy/jPGuc9IK7?= =?Windows-1252?Q?gW5BuTe0N5kL8jLUYITxCqUmjeO7PTlCRTkMu6LYbtHhKH1IPXpuv3dx?= =?Windows-1252?Q?useBfF9azsr06RaKvbkeTj5ryhC6e7cRSBCitgPFCMqQ1mZHwAlgyqek?= =?Windows-1252?Q?Wd/XbjXz9JBgbNxkddnCBIBB2EgXpd3U+WytXh7YiC7anVpb/o9jVOVy?= =?Windows-1252?Q?nXa1WRbHASuAR1ILcpzX16vhbJ61MnK8Qk8Z1lnndbRD2A=3D?= X-Microsoft-Exchange-Diagnostics: 1;BY1PR12MB0439;5:7ZznEawxJNZzGWwkKHLVqvk3v6PEg6WBoQtvda2yi5NPdoMvwMib4NmO/PRiLWveyS5chTn55yJViC5YTeW9vpkkvdLGtWW8S23Dkv2P2PuAxn75KnIDzo8c5JEiZ8mm2/x6Sb7qyApWX2+T5Blcjw==;24:f7T6FKbTqLleC/mkVii/ppCSbjTavnk+o1CBV+1xLOic4ApViJhB8kDJQaTc/343Tz1pZDl7V9RFrAbFEWROHOD+2taiJ3MJUdDjeWlE9CY=;20:nvTvmuwapY5Y2cXshjk+fbU/STdvr9t+zO9p8VYCx9TKWvINA2eXZeQRn+nRiD8TDvd8b+PuN32ZgFPGrkv2OBtE0atfiLlCEvnUJJgHs5QTwSicEBFU8CL8+OhG6dNMDpXdxL5PTzP7GR5vXEoYzISALNZDvfEg1tCowuOfmEuFBIIKvHqdKQ0wQNT4ZcfSYfoJzy4MpWPQEjL9XH4Qr515xxegmWknEvHYe/6GiEkB3sP5X5eBFJNvHCj8l+/K X-OriginatorOrg: amd.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 22 Dec 2015 20:51:17.5376 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-Transport-CrossTenantHeadersStamped: BY1PR12MB0439 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Mika, On 12/18/2015 4:13 AM, Mika Westerberg wrote: > [....] > So instead of this, what if we do not assign dev->get_clk_rate_khz at > all and then do something like below in the core driver? I like the changes below since it is clear to see within the core file how things are handled when get_clk_rate_khz is not assigned (i.e. input_clock_hz = 0), and not necessary relying on the platform driver to return 0 in this case. So, at this point, I can re-submit the V3 and combine these changes, and we both can sign-off. How does that sound? Thanks, Suravee > Of course we still need the other changes you did in this patch to cope > with the missing clock. > > diff --git a/drivers/i2c/busses/i2c-designware-core.c b/drivers/i2c/busses/i2c-designware-core.c > index 8c48b27ba059..25dccd8df772 100644 > --- a/drivers/i2c/busses/i2c-designware-core.c > +++ b/drivers/i2c/busses/i2c-designware-core.c > @@ -271,6 +271,17 @@ static void __i2c_dw_enable(struct dw_i2c_dev *dev, bool enable) > enable ? "en" : "dis"); > } > > +static unsigned long i2c_dw_clk_rate(struct dw_i2c_dev *dev) > +{ > + /* > + * Clock is not necessary if we got LCNT/HCNT values directly from > + * the platform code. > + */ > + if (WARN_ON_ONCE(!dev->get_clk_rate_khz)) > + return 0; > + return dev->get_clk_rate_khz(dev); > +} > + > /** > * i2c_dw_init() - initialize the designware i2c master hardware > * @dev: device private data > @@ -281,7 +292,6 @@ static void __i2c_dw_enable(struct dw_i2c_dev *dev, bool enable) > */ > int i2c_dw_init(struct dw_i2c_dev *dev) > { > - u32 input_clock_khz; > u32 hcnt, lcnt; > u32 reg; > u32 sda_falling_time, scl_falling_time; > @@ -295,8 +305,6 @@ int i2c_dw_init(struct dw_i2c_dev *dev) > } > } > > - input_clock_khz = dev->get_clk_rate_khz(dev); > - > reg = dw_readl(dev, DW_IC_COMP_TYPE); > if (reg == ___constant_swab32(DW_IC_COMP_TYPE_VALUE)) { > /* Configure register endianess access */ > @@ -325,12 +333,12 @@ int i2c_dw_init(struct dw_i2c_dev *dev) > hcnt = dev->ss_hcnt; > lcnt = dev->ss_lcnt; > } else { > - hcnt = i2c_dw_scl_hcnt(input_clock_khz, > + hcnt = i2c_dw_scl_hcnt(i2c_dw_clk_rate(dev), > 4000, /* tHD;STA = tHIGH = 4.0 us */ > sda_falling_time, > 0, /* 0: DW default, 1: Ideal */ > 0); /* No offset */ > - lcnt = i2c_dw_scl_lcnt(input_clock_khz, > + lcnt = i2c_dw_scl_lcnt(i2c_dw_clk_rate(dev), > 4700, /* tLOW = 4.7 us */ > scl_falling_time, > 0); /* No offset */ > @@ -344,12 +352,12 @@ int i2c_dw_init(struct dw_i2c_dev *dev) > hcnt = dev->fs_hcnt; > lcnt = dev->fs_lcnt; > } else { > - hcnt = i2c_dw_scl_hcnt(input_clock_khz, > + hcnt = i2c_dw_scl_hcnt(i2c_dw_clk_rate(dev), > 600, /* tHD;STA = tHIGH = 0.6 us */ > sda_falling_time, > 0, /* 0: DW default, 1: Ideal */ > 0); /* No offset */ > - lcnt = i2c_dw_scl_lcnt(input_clock_khz, > + lcnt = i2c_dw_scl_lcnt(i2c_dw_clk_rate(dev), > 1300, /* tLOW = 1.3 us */ > scl_falling_time, > 0); /* No offset */ >