From nobody Sun Jun 02 21:45:41 2024 X-Original-To: dev-commits-src-main@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 4Vsr5J22cQz5LJB9 for ; Sun, 02 Jun 2024 21:45:56 +0000 (UTC) (envelope-from wlosh@bsdimp.com) Received: from mail-pg1-x52d.google.com (mail-pg1-x52d.google.com [IPv6:2607:f8b0:4864:20::52d]) (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 4Vsr5J0CWdz3xXS for ; Sun, 2 Jun 2024 21:45:56 +0000 (UTC) (envelope-from wlosh@bsdimp.com) Authentication-Results: mx1.freebsd.org; none Received: by mail-pg1-x52d.google.com with SMTP id 41be03b00d2f7-6c8c880f526so735435a12.3 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=Dn/SxfYJfPXCsYGsSSdT32kSwnc+RFfnWMyV8Oot3YF7AqoMV73AMZAHfw4Ofc/9os YilGqGg4wDqLO1rMv+gqGpRdA2xflzyfiQoqwHmZ5/KEJwRhO3p8rZdijdQuKEsVxp98 hq+yZ7MiIABmqHcFi1mXJ/f+knr8HYtPWLr28if4C0rjeeJ4nv1kf/EcVX7gICYjoFbb Mz84e9LEpXgcC5yqDibJ+bSKWKp01JHwne8Rjln1Mc8hTfU1o1ncMmzXAjlhjJFznRwt Wpw0zJIl088oV5feDPYthCiaQvMMzFslo4ov5K8/8k0dA7JFiOpYyMV2JzYS5riuqn4N Kaeg== X-Forwarded-Encrypted: i=1; AJvYcCWqKItEbuB0Id7QTq3zVDhUY3FbaGo5TIpqBqXLOdVTaDBkYfsDMev21yE7fVhJJrdXdW584i2UGilAQ7v83b+x1E18buSdqVqRDvQTARgQ0Q== X-Gm-Message-State: AOJu0YymS5Q6e+E78c/FkgTeekrKBb/zNvAVLLm7oW8tgIqt4XpNU77p X2R8Tw0bc/zbic6ekJdd4nF1iZzWMwAypCnekygreYYojt0bH9TgYRKy5PMaWNEP1JVFMOrvGxk y4TaSHUIGUklqj3t1LMk9u4otkKJzJjJAwsm8vw== 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 the main branch of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-main List-Help: List-Post: List-Subscribe: List-Unsubscribe: X-BeenThere: dev-commits-src-main@freebsd.org Sender: owner-dev-commits-src-main@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: 4Vsr5J0CWdz3xXS --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--