From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752020AbcFCHor (ORCPT ); Fri, 3 Jun 2016 03:44:47 -0400 Received: from mail-pa0-f65.google.com ([209.85.220.65]:33236 "EHLO mail-pa0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750923AbcFCHop (ORCPT ); Fri, 3 Jun 2016 03:44:45 -0400 MIME-Version: 1.0 In-Reply-To: References: <1462980974-22091-1-git-send-email-cedric.madianga@gmail.com> <1462980974-22091-3-git-send-email-cedric.madianga@gmail.com> From: "M'boumba Cedric Madianga" Date: Fri, 3 Jun 2016 09:44:43 +0200 Message-ID: Subject: Re: [PATCH 2/5] i2c: Add STM32F4 I2C driver To: Maxime Coquelin Cc: Patrice Chotard , Wolfram Sang , Rob Herring , Russell King , "linux-arm-kernel@lists.infradead.org" , "devicetree@vger.kernel.org" , "linux-kernel@vger.kernel.org" , linux-i2c@vger.kernel.org Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Maxime, 2016-06-02 18:02 GMT+02:00 Maxime Coquelin : > Hi Cedric, > > 2016-06-02 17:35 GMT+02:00 M'boumba Cedric Madianga : >> Hi, >> >>>> + >>>> +/** >>>> + * stm32f4_i2c_xfer() - Transfer combined I2C message >>>> + * @i2c_adap: Adapter pointer to the controller >>>> + * @msgs: Pointer to data to be written. >>>> + * @num: Number of messages to be executed >>>> + */ >>>> +static int stm32f4_i2c_xfer(struct i2c_adapter *i2c_adap, struct i2c_msg msgs[], >>>> + int num) >>>> +{ >>>> + struct stm32f4_i2c_dev *i2c_dev = i2c_get_adapdata(i2c_adap); >>>> + int ret, i; >>>> + >>>> + i2c_dev->busy = true; >>>> + >>>> + ret = clk_prepare_enable(i2c_dev->clk); >>>> + if (ret) { >>>> + dev_err(i2c_dev->dev, "Failed to prepare_enable clock\n"); >>>> + return ret; >>>> + } >>>> + >>>> + stm32f4_i2c_hw_config(i2c_dev); >>> Maybe you could call this only at probe and resume time? >>> You would save some register accesses. >> Some clarification about this point. >> We need to call stm32f4_i2c_hw_config before each I2C transfer as at >> the end of I2C communication the peripheral is automatically disabled >> and configuration registers are reset. > > Ok, but I wonder how the IP knows this is the last i2c message to be sent? When the IP is enabled we could generated a STOP at any moment by setting a bit in CR1 register. At this moment, the IP knows that this is the last byte to be sent. After STOP generation, I notice that all registers are automatically set with their reset values > Or maybe it gets re-initialized as soon as the clk is disabled? Not really because they are all re-initialized before disabling the clock > > Thanks for the inputs, > Maxime From mboxrd@z Thu Jan 1 00:00:00 1970 From: "M'boumba Cedric Madianga" Subject: Re: [PATCH 2/5] i2c: Add STM32F4 I2C driver Date: Fri, 3 Jun 2016 09:44:43 +0200 Message-ID: References: <1462980974-22091-1-git-send-email-cedric.madianga@gmail.com> <1462980974-22091-3-git-send-email-cedric.madianga@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Return-path: In-Reply-To: Sender: devicetree-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: Maxime Coquelin Cc: Patrice Chotard , Wolfram Sang , Rob Herring , Russell King , "linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org" , "devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" , "linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" , linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org List-Id: devicetree@vger.kernel.org Hi Maxime, 2016-06-02 18:02 GMT+02:00 Maxime Coquelin : > Hi Cedric, > > 2016-06-02 17:35 GMT+02:00 M'boumba Cedric Madianga : >> Hi, >> >>>> + >>>> +/** >>>> + * stm32f4_i2c_xfer() - Transfer combined I2C message >>>> + * @i2c_adap: Adapter pointer to the controller >>>> + * @msgs: Pointer to data to be written. >>>> + * @num: Number of messages to be executed >>>> + */ >>>> +static int stm32f4_i2c_xfer(struct i2c_adapter *i2c_adap, struct i2c_msg msgs[], >>>> + int num) >>>> +{ >>>> + struct stm32f4_i2c_dev *i2c_dev = i2c_get_adapdata(i2c_adap); >>>> + int ret, i; >>>> + >>>> + i2c_dev->busy = true; >>>> + >>>> + ret = clk_prepare_enable(i2c_dev->clk); >>>> + if (ret) { >>>> + dev_err(i2c_dev->dev, "Failed to prepare_enable clock\n"); >>>> + return ret; >>>> + } >>>> + >>>> + stm32f4_i2c_hw_config(i2c_dev); >>> Maybe you could call this only at probe and resume time? >>> You would save some register accesses. >> Some clarification about this point. >> We need to call stm32f4_i2c_hw_config before each I2C transfer as at >> the end of I2C communication the peripheral is automatically disabled >> and configuration registers are reset. > > Ok, but I wonder how the IP knows this is the last i2c message to be sent? When the IP is enabled we could generated a STOP at any moment by setting a bit in CR1 register. At this moment, the IP knows that this is the last byte to be sent. After STOP generation, I notice that all registers are automatically set with their reset values > Or maybe it gets re-initialized as soon as the clk is disabled? Not really because they are all re-initialized before disabling the clock > > Thanks for the inputs, > Maxime -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html From mboxrd@z Thu Jan 1 00:00:00 1970 From: cedric.madianga@gmail.com (M'boumba Cedric Madianga) Date: Fri, 3 Jun 2016 09:44:43 +0200 Subject: [PATCH 2/5] i2c: Add STM32F4 I2C driver In-Reply-To: References: <1462980974-22091-1-git-send-email-cedric.madianga@gmail.com> <1462980974-22091-3-git-send-email-cedric.madianga@gmail.com> Message-ID: To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Hi Maxime, 2016-06-02 18:02 GMT+02:00 Maxime Coquelin : > Hi Cedric, > > 2016-06-02 17:35 GMT+02:00 M'boumba Cedric Madianga : >> Hi, >> >>>> + >>>> +/** >>>> + * stm32f4_i2c_xfer() - Transfer combined I2C message >>>> + * @i2c_adap: Adapter pointer to the controller >>>> + * @msgs: Pointer to data to be written. >>>> + * @num: Number of messages to be executed >>>> + */ >>>> +static int stm32f4_i2c_xfer(struct i2c_adapter *i2c_adap, struct i2c_msg msgs[], >>>> + int num) >>>> +{ >>>> + struct stm32f4_i2c_dev *i2c_dev = i2c_get_adapdata(i2c_adap); >>>> + int ret, i; >>>> + >>>> + i2c_dev->busy = true; >>>> + >>>> + ret = clk_prepare_enable(i2c_dev->clk); >>>> + if (ret) { >>>> + dev_err(i2c_dev->dev, "Failed to prepare_enable clock\n"); >>>> + return ret; >>>> + } >>>> + >>>> + stm32f4_i2c_hw_config(i2c_dev); >>> Maybe you could call this only at probe and resume time? >>> You would save some register accesses. >> Some clarification about this point. >> We need to call stm32f4_i2c_hw_config before each I2C transfer as at >> the end of I2C communication the peripheral is automatically disabled >> and configuration registers are reset. > > Ok, but I wonder how the IP knows this is the last i2c message to be sent? When the IP is enabled we could generated a STOP at any moment by setting a bit in CR1 register. At this moment, the IP knows that this is the last byte to be sent. After STOP generation, I notice that all registers are automatically set with their reset values > Or maybe it gets re-initialized as soon as the clk is disabled? Not really because they are all re-initialized before disabling the clock > > Thanks for the inputs, > Maxime