Re: [PATCH] or1k: Fix compiler warnings

On Wed, Dec 11, 2024 at 11:40:06PM +0100, Corinna Vinschen wrote:
> Hi Stafford,
> 
> On Dec 11 14:39, Stafford Horne wrote:
> > On Wed, Dec 11, 2024 at 02:10:59PM +0100, Corinna Vinschen wrote:
> > > Hi Stafford,
> > > 
> > > I'm not engaged in this stuff, so maybe this is dumb...
> > > 
> > > Changing the parameter from uint32_t to void* sounds like it would have
> > > been the right thing from the start, but it's also an ABI change on 64
> > > bit or1k.  I could imagine the uint32_t is just a pointer value from the
> > > lower 4G space on 64 bit.  Do we even support 64 bit or1k?
> > 
> > Hi Corinna,
> > 
> > I think it's a good conncern.  I put some comments on the patch below with my
> > thoughts.
> > 
> > If this is OK, I will send a v2 with a better description of each change.
> > 
> > See below:
> > [...]
> > I agree that if we did have a 64-bit implementation it may be an
> > issue.  But there never has been one, or1k is only 32-bit.  I think
> > the ABI does not change with this patch.
> 
> IIUC or1k is designed with 32 bit and 64 bit implementations in mind.

Right, the architecture spec was expanded to include 64-bit but that has never
taken off.

> However, if there's no existing 64 bit toolchain for or1k yet, my
> concern doesn't really matter.  Even better if the ABI is 64 bit clean
> before such toolchain is created.
> 
> > Maybe I am wrong, but I think this is safe because:
> > 
> >   - There is no ABI change as void* and uint32_t are the same in or1k
> >   - There is no API change as or1k_uart.h is not a public API
> > 
> > What do you think?
> 
> Sounds good to me.  Just go ahead with a v2 as per your suggestion.

OK, thanks

-Stafford