Date: Sun, 2 Jun 2024 17:45:41 -0400 From: Warner Losh <imp@bsdimp.com> To: Colin Percival <cperciva@tarsnap.com> Cc: Jessica Clarke <jrtc27@freebsd.org>, src-committers <src-committers@freebsd.org>, "<dev-commits-src-all@freebsd.org>" <dev-commits-src-all@freebsd.org>, "<dev-commits-src-main@freebsd.org>" <dev-commits-src-main@freebsd.org> Subject: Re: git: 28aaa58fa64e - main - fu740_pci_dw: Fix PERST delay and keep asserted for rest of reset sequence Message-ID: <CANCZdfp=xgmBcdpKKAo7h1qzaLGPpDuZQDE2A3PakWvFX3T_CQ@mail.gmail.com> In-Reply-To: <0100018fdae2a7fd-b81325da-c255-478f-b7a0-efc0ae77ed43-000000@email.amazonses.com> References: <202406022043.452Khmjb050139@gitrepo.freebsd.org> <0100018fdac22280-97d0bb7c-c35e-4017-aeb8-9c9f2413094c-000000@email.amazonses.com> <6489FC3C-5B0E-4B07-A6EF-92EA3B353423@freebsd.org> <0100018fdae2a7fd-b81325da-c255-478f-b7a0-efc0ae77ed43-000000@email.amazonses.com>
next in thread | previous in thread | raw e-mail | index | archive | help
--000000000000177f780619ef24a8 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Sun, Jun 2, 2024, 3:37=E2=80=AFPM Colin Percival <cperciva@tarsnap.com> = wrote: > On 6/2/24 14:15, Jessica Clarke wrote: > > On 2 Jun 2024, at 22:01, Colin Percival <cperciva@tarsnap.com> wrote: > >> On 6/2/24 13:43, Jessica Clarke wrote: > >>> fu740_pci_dw: Fix PERST delay and keep asserted for rest of rese= t > sequence > >>> DELAY takes microseconds not milliseconds, so 100 was too > low. Moreover, > >>> when enabling hw.pci.clear_pcib, PCI emeration would still stop > at one > >>> of the first bridges, but by asserting PERST for the rest of the > reset > >>> sequence that appears to be reliably addressed. > >> > >> Does this need to be a DELAY as opposed to something asynchronous? We > try to > >> avoid lengthy DELAYs in the boot process. > > > > It=E2=80=99s in the middle of device_attach, so you=E2=80=99d need to b= reak it up into > > two stages. I don=E2=80=99t know if we have a good way of doing that fo= r > > delays; I=E2=80=99ve seen other glue code drivers do things like this, = but > > there may well be a better way, and if so I=E2=80=99m all ears. > > I don't think there's any good mechanism for doing that, unfortunately. > Part > of the problem is that our device probing scheme is designed around the > idea > that by the time you return from device_attach, you know if the device ha= s > successfully attached; if you discover that the device is broken at a lat= er > time it's too late to assign the unit number to a different device. > This is false. We assign unit in probe, and once assigned you don't want to reassign it. I remember talking to Warner about this a while back in the context of nvme= , > but the problem of "the spec says we have to wait a long time and we don'= t > want to do that serially for every disk" was resolved by our driver > learning > to be opportunistic and ask the disks if they were ready yet instead of > simply > waiting the time stipulated by the spec. > If there's no status register to read, you can't do much. And in an SoC there's only going to be one. So between yhe two, i don't see much benefit to be had. And even if we could do this wait asynchronously, there is nothing else to do in parallel. Maybe something to consider when someone decides to write newnewbus. ;-) > Parallel discovery is something we've talked about, but there's a number of logistical issues to sort out first. Warner > Though given > > you won=E2=80=99t have working PCI (so no USB nor NVMe) until this is d= one > > there=E2=80=99s probably not much more you can do during boot whilst yo= u wait, > > so it may not be worth pursuing. Also, given the performance of the SoC > > in question, 100ms isn=E2=80=99t something you=E2=80=99d be close to no= ticing... > > Fair enough, I wasn't sure what sort of hardware this was. Sounds like > we're > fine here; it just caught my eye so I thought I'd ask. > > -- > Colin Percival > FreeBSD Release Engineering Lead & EC2 platform maintainer > Founder, Tarsnap | www.tarsnap.com | Online backups for the truly paranoi= d > --000000000000177f780619ef24a8 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable <div dir=3D"auto"><div><br><br><div class=3D"gmail_quote"><div dir=3D"ltr" = class=3D"gmail_attr">On Sun, Jun 2, 2024, 3:37=E2=80=AFPM Colin Percival &l= t;<a href=3D"mailto:cperciva@tarsnap.com">cperciva@tarsnap.com</a>> wrot= e:<br></div><blockquote class=3D"gmail_quote" style=3D"margin:0 0 0 .8ex;bo= rder-left:1px #ccc solid;padding-left:1ex">On 6/2/24 14:15, Jessica Clarke = wrote:<br> > On 2 Jun 2024, at 22:01, Colin Percival <<a href=3D"mailto:cperciva= @tarsnap.com" target=3D"_blank" rel=3D"noreferrer">cperciva@tarsnap.com</a>= > wrote:<br> >> On 6/2/24 13:43, Jessica Clarke wrote:<br> >>>=C2=A0 =C2=A0 =C2=A0 fu740_pci_dw: Fix PERST delay and keep ass= erted for rest of reset sequence<br> >>>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0DELAY takes microsecon= ds not milliseconds, so 100 was too low. Moreover,<br> >>>=C2=A0 =C2=A0 =C2=A0 when enabling hw.pci.clear_pcib, PCI emera= tion would still stop at one<br> >>>=C2=A0 =C2=A0 =C2=A0 of the first bridges, but by asserting PER= ST for the rest of the reset<br> >>>=C2=A0 =C2=A0 =C2=A0 sequence that appears to be reliably addre= ssed.<br> >><br> >> Does this need to be a DELAY as opposed to something asynchronous?= =C2=A0 We try to<br> >> avoid lengthy DELAYs in the boot process.<br> > <br> > It=E2=80=99s in the middle of device_attach, so you=E2=80=99d need to = break it up into<br> > two stages. I don=E2=80=99t know if we have a good way of doing that f= or<br> > delays; I=E2=80=99ve seen other glue code drivers do things like this,= but<br> > there may well be a better way, and if so I=E2=80=99m all ears.<br> <br> I don't think there's any good mechanism for doing that, unfortunat= ely.=C2=A0 Part<br> of the problem is that our device probing scheme is designed around the ide= a<br> that by the time you return from device_attach, you know if the device has<= br> successfully attached; if you discover that the device is broken at a later= <br> time it's too late to assign the unit number to a different device.<br>= </blockquote></div></div><div dir=3D"auto"><br></div><div dir=3D"auto">This= is false. We assign unit in probe, and once assigned you don't want to= reassign it.=C2=A0</div><div dir=3D"auto"><br></div><div dir=3D"auto"><div= class=3D"gmail_quote"><blockquote class=3D"gmail_quote" style=3D"margin:0 = 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> I remember talking to Warner about this a while back in the context of nvme= ,<br> but the problem of "the spec says we have to wait a long time and we d= on't<br> want to do that serially for every disk" was resolved by our driver le= arning<br> to be opportunistic and ask the disks if they were ready yet instead of sim= ply<br> waiting the time stipulated by the spec.<br></blockquote></div></div><div d= ir=3D"auto"><br></div><div dir=3D"auto">If there's no status register t= o read, you can't do much. And in an SoC there's only going to be o= ne. So between yhe two, i don't see much benefit to be had. And even if= we could do this wait asynchronously, there is nothing else to do in paral= lel.=C2=A0</div><div dir=3D"auto"><br></div><div dir=3D"auto"><div class=3D= "gmail_quote"><blockquote class=3D"gmail_quote" style=3D"margin:0 0 0 .8ex;= border-left:1px #ccc solid;padding-left:1ex"> Maybe something to consider when someone decides to write newnewbus. ;-)<br= ></blockquote></div></div><div dir=3D"auto"><br></div><div dir=3D"auto">Par= allel discovery is something we've talked about, but there's a numb= er of logistical issues to sort out first.</div><div dir=3D"auto"><br></div= ><div dir=3D"auto">Warner=C2=A0</div><div dir=3D"auto"><br></div><div dir= =3D"auto"><div class=3D"gmail_quote"><blockquote class=3D"gmail_quote" styl= e=3D"margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> > Though given<br> > you won=E2=80=99t have working PCI (so no USB nor NVMe) until this is = done<br> > there=E2=80=99s probably not much more you can do during boot whilst y= ou wait,<br> > so it may not be worth pursuing. Also, given the performance of the So= C<br> > in question, 100ms isn=E2=80=99t something you=E2=80=99d be close to n= oticing...<br> <br> Fair enough, I wasn't sure what sort of hardware this was.=C2=A0 Sounds= like we're<br> fine here; it just caught my eye so I thought I'd ask.<br> <br> -- <br> Colin Percival<br> FreeBSD Release Engineering Lead & EC2 platform maintainer<br> Founder, Tarsnap | <a href=3D"http://www.tarsnap.com" rel=3D"noreferrer nor= eferrer" target=3D"_blank">www.tarsnap.com</a> | Online backups for the tru= ly paranoid<br> </blockquote></div></div></div> --000000000000177f780619ef24a8--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CANCZdfp=xgmBcdpKKAo7h1qzaLGPpDuZQDE2A3PakWvFX3T_CQ>