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@xxxxxxxx> 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@xxxxxxxxxxxxx> > > > > 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
