Re: [PATCH 3/3] staging: ccree: simplify ioread/iowrite

[Greg Kroah-Hartman <gregkh@linuxfoundation.org>]

On Mon, Nov 06, 2017 at 10:59:47AM +0200, Gilad Ben-Yossef wrote:
> On Mon, Nov 6, 2017 at 10:37 AM, Tobin C. Harding <me@tobin.cc> wrote:
> > On Mon, Nov 06, 2017 at 06:55:52AM +0000, Gilad Ben-Yossef wrote:
> >> Registers ioread/iowrite operations were done via macros,
> >> sometime using a "magical" implicit parameter.
> >>
> >> Replace all register access with simple inline macros.
> >>
> >> Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>
> >
> > Hi,
> >
> > Nice work. I had a little trouble following this one. Perhaps you are
> > doing more than one thing per patch, feel free to ignore me if I am
> > wrong but it seems you are moving the macro definition of CC_REG to a
> > different header, adding the new inline functions and doing some other
> > change that I can't grok (commented on below).
> >
> > Perhaps this patch could be broken up.
> 
> Thank you Tobin.
> 
> The original macro that I am replacing had an assumption of a variable void *
> cc_base being defined in the context of the macro being called, even though
> it was not listed in the explicit parameter list of the macro.
> 
> The inline function that replace it instead takes an explicit
> parameter a pointer to
> struct ssi_drive data * , who has said cc_base as one of the fields.
> 
> As a result several function that took a void * cc_base parameter
> (which is than only
> used implicitly via the macro without ever being visibly referenced), now take
> struct ssi_drive data * parameter instead which is passed explicitly
> to the inline
> function.
> 
> These seems to be the places you are referring to. They are cascading changes
> resulting from the change in API between the macro and the inline
> function that replaces it.
> 
> I imagine I can try to break that change to two patches but at least
> in my mind this is artificial
> and it is a single logical change.
> 
> Having said that, if you think otherwise and consider this
> none-reviewable even after this
> explanation let me know and  I'd be happy to break it down.

Nah, this is fine, I'll take it as-is.  Tobin, thanks for the review.

greg k-h