Re: [PATCH] drivers/tty/serial/8250: Make Aspeed VUART SIRQ polarity configurable

Andrew Jeffery andrew at aj.id.au
Mon Jul 29 13:02:57 AEST 2019



On Mon, 29 Jul 2019, at 11:51, Oskar Senft wrote:
> 
> Hi Jeremy
> 
> > Somewhat offtopic, but are you sure you want to enable the SuperIO
> >  device?
> No :-D I'm aware of CVE-2019-6260. I just listed it as a potential 
> source of SIRQs.
> > > The VUART is covered by this code and we don't have a PUART driver
> >  > yet.
> >  > 
> >  > It might make sense to have this as a global setting which each driver
> >  > could read. But wouldn't this be an exercise for the future where we
> >  > actually have a second device? I don't think the Aspeed currently has
> >  > any other devices that could generate a SIRQ (except for the PUART for
> >  > which there's no driver).
> > 
> >  We have a bunch of SIRQ sources that we use (on OpenPOWER platforms) -
> >  the BT interface, the KCS interface, the UARTs, and the mbox hardware.
> >  It's not just the VUART/PUART :)
> Interesting. From what Aspeed explained, the SIRQ polarity for UARTs is 
> inverted to that for other devices. I haven't looked into how other 
> devices work, to be honest. Do we set the polarity there explicitly?
> > > Having said that, ideally I'd like the SIRQ polarity to be auto-
> >  > configured from the LPC/eSPI HW pin strap anyway. I have the code for
> >  > that almost done. Maybe we shouldn't even have the sysfs interface for
> >  > it and I should strip that out?
> > 
> >  Yeah, I think I agree with that. The only downside is that the 
> >  individual drivers will now need to poke at the SCU to query the
> >  LPC/eSPI configuration. If you can find a nice way to do that, then that
> >  sounds like the best approach. Can you query it through the parent bus
> >  perhaps?
> I'm experimenting with this:
> syscon_regmap_lookup_by_compatible("aspeed,ast2500-scu");
> 
> I'll need to think on how to make this work on both ast2400 and 
> ast2500, though. I actually need to have a look at the ast2400 data 
> sheet wrt. VUART registers, too!
> 
> The structure is this:
> apb {
>  syscon {
>  ...
>  }
>  vuart {
>  ...
>  }
> }
> 
> So the vuart is a sibling of syscon, which holds the registers. I guess 
> I'll have to go with "by_compatible"?

It might be better to add a property to the UART DT node that references
the SCU syscon by phandle, that way you don't need to muck around
with compatible names for the SCU syscon.

Andrew


More information about the Linux-aspeed mailing list