All of lore.kernel.org
 help / color / mirror / Atom feed
From: Laxman Dewangan <ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
To: Wolfram Sang <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
Cc: "khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org"
	<khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org>,
	Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>,
	"linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org"
	<linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	"linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org"
	<linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	"linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org"
	<linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	"olof-nZhT3qVonbNeoWH0uzbU5w@public.gmane.org"
	<olof-nZhT3qVonbNeoWH0uzbU5w@public.gmane.org>
Subject: Re: [PATCH REBASE 1/2] i2c: tegra: I2_M_NOSTART functionality not supported in Tegra20
Date: Sat, 18 Aug 2012 18:20:11 +0530	[thread overview]
Message-ID: <502F8F83.5030902@nvidia.com> (raw)
In-Reply-To: <20120818124703.GC12839-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>

Thanks for review.

On Saturday 18 August 2012 06:17 PM, Wolfram Sang wrote:
> * PGP Signed by an unknown key
>
> On Sat, Aug 18, 2012 at 12:32:34AM +0530, Laxman Dewangan wrote:
>> +	bool has_continue_xfer_support;
> I wonder if it makes sense to carry a pointer here to the
> tegra_i2c_hw_feature in use instead of copying all entries by hand,
> since they might get more and more.
>

Ok, we can store the hw pointer in device structure. I will send the new 
patch.

>>   };
>>
>>   static void dvc_writel(struct tegra_i2c_dev *i2c_dev, u32 val, unsigned long reg)
>> @@ -563,7 +574,17 @@ static int tegra_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[],
>>   	if (i2c_dev->is_suspended)
>>   		return -EBUSY;
>>
>> -	clk_prepare_enable(i2c_dev->div_clk);
>> +	/* Support I2C_M_NOSTART only if HW support continue xfer. */
>> +	for (i = 0; i<  num - 1; i++) {
>> +		if ((msgs[i + 1].flags&  I2C_M_NOSTART)&&
>> +			!i2c_dev->has_continue_xfer_support) {
>> +			dev_err(i2c_dev->dev,
>> +				"mesg %d have illegal flag\n", i + 1);
>> +			return -EINVAL;
>> +		}
>> +	}
> Drivers are requested to explicitly check for features of the I2C bus
> (like M_NOSTART) before using them, so I'd skip this extra check.
>

Ok, I kept this as part of flag checking so illegal flag should not be 
passed. I will remove this on next version patch.

>> +
>> +	clk_prepare_enable(i2c_dev->clk);
>  From a glimpse, this change looks unrelated at least. Even wrong, no?
>

It was already there, just before above check. Due to insertion of 
check, this code shifted, otherwise it is not a new code.

WARNING: multiple messages have this Message-ID (diff)
From: Laxman Dewangan <ldewangan@nvidia.com>
To: Wolfram Sang <w.sang@pengutronix.de>
Cc: "khali@linux-fr.org" <khali@linux-fr.org>,
	Stephen Warren <swarren@nvidia.com>,
	"linux-i2c@vger.kernel.org" <linux-i2c@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-tegra@vger.kernel.org" <linux-tegra@vger.kernel.org>,
	"olof@lixom.net" <olof@lixom.net>
Subject: Re: [PATCH REBASE 1/2] i2c: tegra: I2_M_NOSTART functionality not supported in Tegra20
Date: Sat, 18 Aug 2012 18:20:11 +0530	[thread overview]
Message-ID: <502F8F83.5030902@nvidia.com> (raw)
In-Reply-To: <20120818124703.GC12839@pengutronix.de>

Thanks for review.

On Saturday 18 August 2012 06:17 PM, Wolfram Sang wrote:
> * PGP Signed by an unknown key
>
> On Sat, Aug 18, 2012 at 12:32:34AM +0530, Laxman Dewangan wrote:
>> +	bool has_continue_xfer_support;
> I wonder if it makes sense to carry a pointer here to the
> tegra_i2c_hw_feature in use instead of copying all entries by hand,
> since they might get more and more.
>

Ok, we can store the hw pointer in device structure. I will send the new 
patch.

>>   };
>>
>>   static void dvc_writel(struct tegra_i2c_dev *i2c_dev, u32 val, unsigned long reg)
>> @@ -563,7 +574,17 @@ static int tegra_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[],
>>   	if (i2c_dev->is_suspended)
>>   		return -EBUSY;
>>
>> -	clk_prepare_enable(i2c_dev->div_clk);
>> +	/* Support I2C_M_NOSTART only if HW support continue xfer. */
>> +	for (i = 0; i<  num - 1; i++) {
>> +		if ((msgs[i + 1].flags&  I2C_M_NOSTART)&&
>> +			!i2c_dev->has_continue_xfer_support) {
>> +			dev_err(i2c_dev->dev,
>> +				"mesg %d have illegal flag\n", i + 1);
>> +			return -EINVAL;
>> +		}
>> +	}
> Drivers are requested to explicitly check for features of the I2C bus
> (like M_NOSTART) before using them, so I'd skip this extra check.
>

Ok, I kept this as part of flag checking so illegal flag should not be 
passed. I will remove this on next version patch.

>> +
>> +	clk_prepare_enable(i2c_dev->clk);
>  From a glimpse, this change looks unrelated at least. Even wrong, no?
>

It was already there, just before above check. Due to insertion of 
check, this code shifted, otherwise it is not a new code.


  parent reply	other threads:[~2012-08-18 12:50 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-08-17 19:02 [PATCH REBASE 1/2] i2c: tegra: I2_M_NOSTART functionality not supported in Tegra20 Laxman Dewangan
2012-08-17 19:02 ` Laxman Dewangan
     [not found] ` <1345230155-4252-1-git-send-email-ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
2012-08-17 19:02   ` [PATCH REBASE 2/2] i2c: tegra: dynamically control fast clk Laxman Dewangan
2012-08-17 19:02     ` Laxman Dewangan
     [not found]     ` <1345230155-4252-2-git-send-email-ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
2012-08-18 18:52       ` Wolfram Sang
2012-08-18 18:52         ` Wolfram Sang
2012-08-17 21:35   ` [PATCH REBASE 1/2] i2c: tegra: I2_M_NOSTART functionality not supported in Tegra20 Stephen Warren
2012-08-17 21:35     ` Stephen Warren
2012-08-18 12:47   ` Wolfram Sang
2012-08-18 12:47     ` Wolfram Sang
     [not found]     ` <20120818124703.GC12839-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
2012-08-18 12:50       ` Laxman Dewangan [this message]
2012-08-18 12:50         ` Laxman Dewangan
     [not found]         ` <502F8F83.5030902-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
2012-08-18 18:29           ` Wolfram Sang
2012-08-18 18:29             ` Wolfram Sang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=502F8F83.5030902@nvidia.com \
    --to=ldewangan-ddmlm1+adcrqt0dzr+alfa@public.gmane.org \
    --cc=khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org \
    --cc=linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=olof-nZhT3qVonbNeoWH0uzbU5w@public.gmane.org \
    --cc=swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org \
    --cc=w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.