Re: [PATCH v4 8/9] MIPS: SGI-IP27: fix readb/writeb addressing
On Tue, 13 Aug 2019 10:47:13 +0200
Philippe Mathieu-Daudé wrote:
> Hi Thomas,
>
> On 8/11/19 9:29 AM, Greg Kroah-Hartman wrote:
> > On Sat, Aug 10, 2019 at 04:22:23PM +0300, Andy Shevchenko wrote:
> >> On Fri, Aug 9, 2019 at 1:34 PM Thomas Bogendoerfer
> >> wrote:
> >>>
> >>> Our chosen byte swapping, which is what firmware already uses, is to
> >>> do readl/writel by normal lw/sw intructions (data invariance). This
> >>> also means we need to mangle addresses for u8 and u16 accesses. The
> >>> mangling for 16bit has been done aready, but 8bit one was missing.
> >>> Correcting this causes different addresses for accesses to the
> >>> SuperIO and local bus of the IOC3 chip. This is fixed by changing
> >>> byte order in ioc3 and m48rtc_rtc structs.
> >>
> >>> /* serial port register map */
> >>> struct ioc3_serialregs {
> >>> - uint32_tsscr;
> >>> - uint32_tstpir;
> >>> - uint32_tstcir;
> >>> - uint32_tsrpir;
> >>> - uint32_tsrcir;
> >>> - uint32_tsrtr;
> >>> - uint32_tshadow;
> >>> + u32 sscr;
> >>> + u32 stpir;
> >>> + u32 stcir;
> >>> + u32 srpir;
> >>> + u32 srcir;
> >>> + u32 srtr;
> >>> + u32 shadow;
> >>> };
> >>
> >> Isn't it a churn? AFAIU kernel documentation the uint32_t is okay to
> >> use, just be consistent inside one module / driver.
> >> Am I mistaken?
> >
> > No, but really it uint* shouldn't be used anywhere in the kernel source
> > as it does not make sense.
>
> If you respin your series, please send this cleanup as a separate patch.
no need for an extra patch. I realized that patch 7 in this series introduces
all of these uint32_t. So i already fixed it there.
Thomas.
--
SUSE Linux GmbH
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah
HRB 21284 (AG Nürnberg)
Re: [PATCH v4 8/9] MIPS: SGI-IP27: fix readb/writeb addressing
Hi Thomas,
On 8/11/19 9:29 AM, Greg Kroah-Hartman wrote:
> On Sat, Aug 10, 2019 at 04:22:23PM +0300, Andy Shevchenko wrote:
>> On Fri, Aug 9, 2019 at 1:34 PM Thomas Bogendoerfer
>> wrote:
>>>
>>> Our chosen byte swapping, which is what firmware already uses, is to
>>> do readl/writel by normal lw/sw intructions (data invariance). This
>>> also means we need to mangle addresses for u8 and u16 accesses. The
>>> mangling for 16bit has been done aready, but 8bit one was missing.
>>> Correcting this causes different addresses for accesses to the
>>> SuperIO and local bus of the IOC3 chip. This is fixed by changing
>>> byte order in ioc3 and m48rtc_rtc structs.
>>
>>> /* serial port register map */
>>> struct ioc3_serialregs {
>>> - uint32_tsscr;
>>> - uint32_tstpir;
>>> - uint32_tstcir;
>>> - uint32_tsrpir;
>>> - uint32_tsrcir;
>>> - uint32_tsrtr;
>>> - uint32_tshadow;
>>> + u32 sscr;
>>> + u32 stpir;
>>> + u32 stcir;
>>> + u32 srpir;
>>> + u32 srcir;
>>> + u32 srtr;
>>> + u32 shadow;
>>> };
>>
>> Isn't it a churn? AFAIU kernel documentation the uint32_t is okay to
>> use, just be consistent inside one module / driver.
>> Am I mistaken?
>
> No, but really it uint* shouldn't be used anywhere in the kernel source
> as it does not make sense.
If you respin your series, please send this cleanup as a separate patch.
Thanks,
Phil.
Re: [PATCH v4 8/9] MIPS: SGI-IP27: fix readb/writeb addressing
On Sat, Aug 10, 2019 at 04:22:23PM +0300, Andy Shevchenko wrote:
> On Fri, Aug 9, 2019 at 1:34 PM Thomas Bogendoerfer
> wrote:
> >
> > Our chosen byte swapping, which is what firmware already uses, is to
> > do readl/writel by normal lw/sw intructions (data invariance). This
> > also means we need to mangle addresses for u8 and u16 accesses. The
> > mangling for 16bit has been done aready, but 8bit one was missing.
> > Correcting this causes different addresses for accesses to the
> > SuperIO and local bus of the IOC3 chip. This is fixed by changing
> > byte order in ioc3 and m48rtc_rtc structs.
>
> > /* serial port register map */
> > struct ioc3_serialregs {
> > - uint32_tsscr;
> > - uint32_tstpir;
> > - uint32_tstcir;
> > - uint32_tsrpir;
> > - uint32_tsrcir;
> > - uint32_tsrtr;
> > - uint32_tshadow;
> > + u32 sscr;
> > + u32 stpir;
> > + u32 stcir;
> > + u32 srpir;
> > + u32 srcir;
> > + u32 srtr;
> > + u32 shadow;
> > };
>
> Isn't it a churn? AFAIU kernel documentation the uint32_t is okay to
> use, just be consistent inside one module / driver.
> Am I mistaken?
No, but really it uint* shouldn't be used anywhere in the kernel source
as it does not make sense.
thanks,
greg k-h
Re: [PATCH v4 8/9] MIPS: SGI-IP27: fix readb/writeb addressing
On Fri, Aug 9, 2019 at 1:34 PM Thomas Bogendoerfer
wrote:
>
> Our chosen byte swapping, which is what firmware already uses, is to
> do readl/writel by normal lw/sw intructions (data invariance). This
> also means we need to mangle addresses for u8 and u16 accesses. The
> mangling for 16bit has been done aready, but 8bit one was missing.
> Correcting this causes different addresses for accesses to the
> SuperIO and local bus of the IOC3 chip. This is fixed by changing
> byte order in ioc3 and m48rtc_rtc structs.
> /* serial port register map */
> struct ioc3_serialregs {
> - uint32_tsscr;
> - uint32_tstpir;
> - uint32_tstcir;
> - uint32_tsrpir;
> - uint32_tsrcir;
> - uint32_tsrtr;
> - uint32_tshadow;
> + u32 sscr;
> + u32 stpir;
> + u32 stcir;
> + u32 srpir;
> + u32 srcir;
> + u32 srtr;
> + u32 shadow;
> };
Isn't it a churn? AFAIU kernel documentation the uint32_t is okay to
use, just be consistent inside one module / driver.
Am I mistaken?
--
With Best Regards,
Andy Shevchenko
