RE: [PATCH v7 2/5] OMAP: McBSP: Modify macros/functions API for easy cache access

From: "Varadarajan, Charu Latha" <charu@ti.com>
To: Janusz Krzysztofik <jkrzyszt@tis.icnet.pl>
Cc: Tony Lindgren <tony@atomide.com>,
	Jarkko Nikula <jhnikula@gmail.com>,
	Peter Ujfalusi <peter.ujfalusi@nokia.com>,
	"linux-omap@vger.kernel.org" <linux-omap@vger.kernel.org>
Subject: RE: [PATCH v7 2/5] OMAP: McBSP: Modify macros/functions API for easy cache access
Date: Mon, 14 Dec 2009 11:35:06 +0530	[thread overview]
Message-ID: <EAF47CD23C76F840A9E7FCE10091EFAB02BC141E94@dbde02.ent.ti.com> (raw)
In-Reply-To: <200912111642.34458.jkrzyszt@tis.icnet.pl>

<snip>

>> >@@ -612,7 +597,6 @@ EXPORT_SYMBOL(omap_mcbsp_stop);
>> > int omap_mcbsp_pollwrite(unsigned int id, u16 buf)
>> > {
>> > 	struct omap_mcbsp *mcbsp;
>> >-	void __iomem *base;
>> >
>> > 	if (!omap_mcbsp_check_valid_id(id)) {
>> > 		printk(KERN_ERR "%s: Invalid id (%d)\n",
>> >__func__, id + 1);
>> >@@ -620,28 +604,25 @@ int omap_mcbsp_pollwrite(unsigned int id
>> > 	}
>> >
>> > 	mcbsp = id_to_mcbsp_ptr(id);
>> >-	base = mcbsp->io_base;
>> >
>> >-	OMAP_MCBSP_WRITE(base, DXR1, buf);
>> >+	MCBSP_WRITE(mcbsp, DXR1, buf);
>>
>> OMAP3/4 allows 32 bit data access also.
>> How is it taken care?
>
>Not at all in case of those several particular changes above. 
>I'm pretty sure 
>that just replacing old OMAP_MCBSP_WRITE(base, ...) with new 
>MCBSP_WRITE(mcbsp, ...) changed nothing in terms of OMAP3/4 
>32-bit data 
>access. Again, beyond the scope.
>
>> Please refer to 
>> http://patchwork.kernel.org/patch/54896/ for more details.
>>
>> Quoting Tony's words:
>> "To me it smells like the all drivers using the the
>> omap_mcbsp_pollread/write are broken legacy drivers.
>> So maybe we should just remove these two functions?
>> If we really want to keep these around, we should review
>> the drivers using these functions. "
>
>Please have a look at patch 1/5, there readw()/writew() has 
>been replaced with 
>OMAP_MCBSP_READ()/_WRITE() inside 
>omap_mcbsp_pollwrite()/_pollread(). If you 
>think there is something wrong with it, please comment that 
>patch, not this 
>one that changes nothing related here, I believe.

The point here is to review the driver before pushing any patch
touching these functions.

<snip>