From nobody Sun Jun 02 21:45:41 2024 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4Vsr5J4Lp6z5LJ7Y for ; Sun, 02 Jun 2024 21:45:56 +0000 (UTC) (envelope-from wlosh@bsdimp.com) Received: from mail-pf1-x42b.google.com (mail-pf1-x42b.google.com [IPv6:2607:f8b0:4864:20::42b]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "smtp.gmail.com", Issuer "WR4" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4Vsr5H4qK1z3xBP for ; Sun, 2 Jun 2024 21:45:55 +0000 (UTC) (envelope-from wlosh@bsdimp.com) Authentication-Results: mx1.freebsd.org; none Received: by mail-pf1-x42b.google.com with SMTP id d2e1a72fcca58-70264bcb631so511033b3a.2 for ; Sun, 02 Jun 2024 14:45:55 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bsdimp-com.20230601.gappssmtp.com; s=20230601; t=1717364754; x=1717969554; darn=freebsd.org; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=qKbSMcF/7cx+xKspk3F6qSb3zNzW5tWriYUsVNgH8OA=; b=r2P5BOYmq2AZiWDUO9yCoeXgCL1bQo5jgO5XgnQESjFwsK6obn3o60GDx1QQs8zIL8 h7C7jqWa8Fb3dP2yravQ08Vyg1fDwEn81e9CAQXvc4RPfBhrHpoyaUwVvMbIiQpoaW+J AtVL7Nhpi8Z7dMhI0hnf+QaJI+dy9Niie0p8W85wZHxplTHwHEUduLFD/k6JO66qZKRq XyIHJ36a73qhESbqKat6GH7zFpnzJ6OT1y0L4N8T2PmJqq0UW2gseZ+iE6j5zGs9hY2+ KKS8qHLfF8IQOgD/USmETR8NIn0gm7wBz7qUOY0E1Q9aoHmVC26P5mD6en4VT2lEKDyI /h1g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1717364754; x=1717969554; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=qKbSMcF/7cx+xKspk3F6qSb3zNzW5tWriYUsVNgH8OA=; b=b8zaTAurzaYxPe+5qyrtPyuxYbDmo8IowZoBm9rfwersGaluSqLwWrzhyVutpEuOKl SNZZgvT3YBBtBPNTQY5irJmxko+nuHNhoAgtQZQqhWr+8Zd03110JMsoyX2UF656x4gJ WO3se6KIYVXVlrSCq1CayVUPJm2IvQ49lL3dh9s13+0cSmcl5JMYL1SbsN1lRAwbZU2E tBZOsuuJ8T6lb7utMkcYLT3phNDcr3Tg+/YFc8nKziPQMvcOdtNKtc3TVpdJ5XOpjVEf MAgtbxKOgdP685UTjTzcFd0tUz7tSR9/3SF91vudYGAbfcEexiuqkt+Di/ousOd1gvc7 M5mg== X-Forwarded-Encrypted: i=1; AJvYcCWwJA5U9RY3Qsga3JNys1kHGQDyot745/3hpDoPA9gmZgc5yz/vlBBmo+XERqyLhyfqci/7T9lV/bKSqG0atbZGKu4TqbEjsEy204vS8obt X-Gm-Message-State: AOJu0YyZMaaryi4CA59WvCRcaay0wTzngLdeS8csJydzMGxwUVhIpxWH up/HA4rZklF4/82QQDnQta6JkVflAlUgkKfqkFMHlK6VzP1x21YAloUjNl2Vcn9VT9wBEJuVGI2 QbcD6pPOmjb8KIoga8waUkfeSAHczcGQoDDzDLA== X-Google-Smtp-Source: AGHT+IHe/elu18YzGvzqQwn/sJR58xLYQEfX2+VyUhslNW08R3m21/MnGQh+IIW5nCjpU1EQ1Cht6Cv/gmv4DKjC2sE= X-Received: by 2002:a05:6a20:9716:b0:1af:a9ad:fbb9 with SMTP id adf61e73a8af0-1b26f29dfa3mr5989049637.59.1717364753653; Sun, 02 Jun 2024 14:45:53 -0700 (PDT) List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: X-BeenThere: dev-commits-src-all@freebsd.org Sender: owner-dev-commits-src-all@FreeBSD.org MIME-Version: 1.0 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> In-Reply-To: <0100018fdae2a7fd-b81325da-c255-478f-b7a0-efc0ae77ed43-000000@email.amazonses.com> From: Warner Losh Date: Sun, 2 Jun 2024 17:45:41 -0400 Message-ID: Subject: Re: git: 28aaa58fa64e - main - fu740_pci_dw: Fix PERST delay and keep asserted for rest of reset sequence To: Colin Percival Cc: Jessica Clarke , src-committers , "" , "" Content-Type: multipart/alternative; boundary="000000000000177f780619ef24a8" X-Spamd-Bar: ---- X-Rspamd-Pre-Result: action=no action; module=replies; Message is reply to one we originated X-Spamd-Result: default: False [-4.00 / 15.00]; REPLY(-4.00)[]; ASN(0.00)[asn:15169, ipnet:2607:f8b0::/32, country:US] X-Rspamd-Queue-Id: 4Vsr5H4qK1z3xBP --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 = wrote: > On 6/2/24 14:15, Jessica Clarke wrote: > > On 2 Jun 2024, at 22:01, Colin Percival 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


On Sun, Jun 2, 2024, 3:37=E2=80=AFPM Colin Percival &l= t;cperciva@tarsnap.com> wrot= e:
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:
>>>=C2=A0 =C2=A0 =C2=A0 fu740_pci_dw: Fix PERST delay and keep ass= erted for rest of reset sequence
>>>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0DELAY takes microsecon= ds not milliseconds, so 100 was too low. Moreover,
>>>=C2=A0 =C2=A0 =C2=A0 when enabling hw.pci.clear_pcib, PCI emera= tion would still stop at one
>>>=C2=A0 =C2=A0 =C2=A0 of the first bridges, but by asserting PER= ST for the rest of the reset
>>>=C2=A0 =C2=A0 =C2=A0 sequence that appears to be reliably addre= ssed.
>>
>> Does this need to be a DELAY as opposed to something asynchronous?= =C2=A0 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 = break it up into
> two stages. I don=E2=80=99t know if we have a good way of doing that f= or
> 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, unfortunat= ely.=C2=A0 Part
of the problem is that our device probing scheme is designed around the ide= a
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=
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.=C2=A0

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 d= on't
want to do that serially for every disk" was resolved by our driver le= arning
to be opportunistic and ask the disks if they were ready yet instead of sim= ply
waiting the time stipulated by the spec.

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

Maybe something to consider when someone decides to write newnewbus. ;-)

Par= allel discovery is something we've talked about, but there's a numb= er of logistical issues to sort out first.

Warner=C2=A0

> Though given
> you won=E2=80=99t have working PCI (so no USB nor NVMe) until this is = done
> there=E2=80=99s probably not much more you can do during boot whilst y= ou wait,
> so it may not be worth pursuing. Also, given the performance of the So= C
> in question, 100ms isn=E2=80=99t something you=E2=80=99d be close to n= oticing...

Fair enough, I wasn't sure what sort of hardware this was.=C2=A0 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 tru= ly paranoid
--000000000000177f780619ef24a8--