Re: [PATCH 3/4] OMAP3 and 4 i2c mark extended reg enums as extended only

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 3/4/2011 9:32 AM, Andy Green wrote:
On 03/03/2011 09:33 PM, Somebody in the thread at some point said:

Hi -

Since it is a patch on the I2C driver, the subject should start with
something like "I2C: OMAP2+: XXXXX". That comment is also applicable for
the other patches of the series except the first one.

This patch changes the extended register name to make it clearer
they only exist in OMAP4 context

Cc: patches@xxxxxxxxxx
Reported-by: Peter Maydell<peter.maydell@xxxxxxxxxx>
Signed-off-by: Andy Green<andy.green@xxxxxxxxxx>

The I2C maintainer should be in CC as well.

OK thanks for this correction.

+ /* only on OMAP4430 */
+ OMAP_I2C_OMAP4430_REVNB_LO,
+ OMAP_I2C_OMAP4430_REVNB_HI,
+ OMAP_I2C_OMAP4430_IRQSTATUS_RAW,
+ OMAP_I2C_OMAP4430_IRQENABLE_SET,

I think that you should keep only the comment, because it is not really
recommended to add SoC related information directly in IP register names.
These new registers are just an evolution of the I2C IP. The first
instances of that version are used in OMAP4 first, but OMAP4 variants
(4440) and OMAP5 will use the same one.

Bottom line is that we can probably drop that patch from the series.

The desire of this patch is to make it clear to the eye that a register
that was introduced in what we will now call "IP_V2" is being touched.
That is good because then code like

	if (dev->rev == BLAH_IP_V1)
		touch(BLAH_BLAH_IP_V2);

will stand out clearly as wrong.  So I will update the patch rather than
drop it, since the IP_Vn scheme is a much better fit for what is
actually being done.  If you still don't like it we can forget about it
then.

It is a little bit better. I personally don't think it is necessary, but since it is a purely subjective opinion, you can go ahead with that fix.

Regards,
Benoit
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Arm (vger)]     [ARM Kernel]     [ARM MSM]     [Linux Tegra]     [Linux WPAN Networking]     [Linux Wireless Networking]     [Maemo Users]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux