* Alan Cox <alan@xxxxxxxxxxxxxxx> wrote:
> > Please Cc: x86 patches to the x86 maintainers.
>
> Sure - I should probably have also cc'd Feng - I've forwarded it and
> added Feng so it doesn't get missed.
thx.
> > > +static int hsu_inited;
> >
> > 'initialized' is the proper English word i think.
>
> Then we should probably run /sbin/initialize in future ;) [...]
Fortunately there's no /sbin/inited.
> [...] inited is perfectly fine computerspeak and much less typing.
It's a distinctly annoying grammar mistake (to me at least) and it only
comes up very rarely in the kernel - which has its fair share of
annoying grammar otherwise.
As per a quick & dirty 'git grep' run we have 4267 (96%) instances of
'initialized' and only 174 (4%) 'inited' instances usage right now.
hsu_init_done might be a compromise?
But ... no strong feelings in any case. If you the native speaker are
not annoyed by reading 'inited' then i guess i'm in the minority.
> > > +static void early_hsu_init(void)
> > > +{
> > > + u8 lcr;
> > > +
> > > + if (phsu && hsu_inited)
> > > + return;
> >
> > Surely one of those will suffice as a "have we initialized" flag?
> >
> > Also, under what circumstances can we call early_hsu_init() twice?
>
> Don't think we can
>
> > > + /* GPIO workaround */
> > > + set_fixmap_nocache(FIX_EARLYCON_MEM_BASE,
> > > MFD_GPIO_HSU_REG);
> > > + phsu = (void *)(__fix_to_virt(FIX_EARLYCON_MEM_BASE) +
> > > + (MFD_GPIO_HSU_REG & (PAGE_SIZE - 1)));
> > > +
> > > + *((u32 *)phsu) = 0x55465;
> >
> > What does 0x55465 stand for?
>
> It's a firmware fixup. Its a magic value (even to most of us who work
> here ;)). Feng - am I right in thinking we don't need that anyway with
> the current firmware ?
If it's some magic number it's worth symbolizing it - keeping future
generations of happy mrst hackers from wondering at that incantation.
>
> > > +{
> > > + unsigned int timeout = 10000; /* 10ms*/
> > > + u8 status;
> > > +
> > > + while (timeout--) {
> > > + status = readb(phsu + UART_LSR);
> > > + if (status & BOTH_EMPTY)
> > > + break;
> > > +
> > > + udelay(1);
> > > + }
> > > +
> > > + if (timeout == 0xffffffff)
> > > + return;
> >
> > Using the -1 literal will dtrt too, and will be slightly clearer to
> > the potentially overworked reader of such patches.
>
> If they have a degree in C sign propagation.
My 8yo son knows it that the next number below zero is -1, and he
distinctly has no degree in C sign propagation rules. But you are right
to point out the correct solution:
> [...] Better is probably
>
> while (--timeout) {..
> }
indeed.
Thanks,
Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
