<div dir="ltr">Done. I submitted just a v2 of the patch alongside the other bits and pieces that are required to make this work.<div><br></div><div>Let me know if that works for you or if there's anything that should be changed.</div><div><br></div><div>Oskar.</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Sun, Jul 28, 2019 at 11:02 PM Andrew Jeffery <<a href="mailto:andrew@aj.id.au">andrew@aj.id.au</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><br>
<br>
On Mon, 29 Jul 2019, at 11:51, Oskar Senft wrote:<br>
> <br>
> Hi Jeremy<br>
> <br>
> > Somewhat offtopic, but are you sure you want to enable the SuperIO<br>
> >  device?<br>
> No :-D I'm aware of CVE-2019-6260. I just listed it as a potential <br>
> source of SIRQs.<br>
> > > The VUART is covered by this code and we don't have a PUART driver<br>
> >  > yet.<br>
> >  > <br>
> >  > It might make sense to have this as a global setting which each driver<br>
> >  > could read. But wouldn't this be an exercise for the future where we<br>
> >  > actually have a second device? I don't think the Aspeed currently has<br>
> >  > any other devices that could generate a SIRQ (except for the PUART for<br>
> >  > which there's no driver).<br>
> > <br>
> >  We have a bunch of SIRQ sources that we use (on OpenPOWER platforms) -<br>
> >  the BT interface, the KCS interface, the UARTs, and the mbox hardware.<br>
> >  It's not just the VUART/PUART :)<br>
> Interesting. From what Aspeed explained, the SIRQ polarity for UARTs is <br>
> inverted to that for other devices. I haven't looked into how other <br>
> devices work, to be honest. Do we set the polarity there explicitly?<br>
> > > Having said that, ideally I'd like the SIRQ polarity to be auto-<br>
> >  > configured from the LPC/eSPI HW pin strap anyway. I have the code for<br>
> >  > that almost done. Maybe we shouldn't even have the sysfs interface for<br>
> >  > it and I should strip that out?<br>
> > <br>
> >  Yeah, I think I agree with that. The only downside is that the <br>
> >  individual drivers will now need to poke at the SCU to query the<br>
> >  LPC/eSPI configuration. If you can find a nice way to do that, then that<br>
> >  sounds like the best approach. Can you query it through the parent bus<br>
> >  perhaps?<br>
> I'm experimenting with this:<br>
> syscon_regmap_lookup_by_compatible("aspeed,ast2500-scu");<br>
> <br>
> I'll need to think on how to make this work on both ast2400 and <br>
> ast2500, though. I actually need to have a look at the ast2400 data <br>
> sheet wrt. VUART registers, too!<br>
> <br>
> The structure is this:<br>
> apb {<br>
>  syscon {<br>
>  ...<br>
>  }<br>
>  vuart {<br>
>  ...<br>
>  }<br>
> }<br>
> <br>
> So the vuart is a sibling of syscon, which holds the registers. I guess <br>
> I'll have to go with "by_compatible"?<br>
<br>
It might be better to add a property to the UART DT node that references<br>
the SCU syscon by phandle, that way you don't need to muck around<br>
with compatible names for the SCU syscon.<br>
<br>
Andrew<br>
</blockquote></div>