On 10/29/25 4:26 AM, Danilo Krummrich wrote:
> On Wed Oct 29, 2025 at 4:03 AM CET, John Hubbard wrote:
...
>> + fn try_from(boot0: regs::NV_PMC_BOOT_0) -> Result<Self> {
>> + Ok(Self {
>> + chipset: boot0.chipset()?,
>> + revision: Revision {
>> + major: boot0.major_revision(),
>> + minor: boot0.minor_revision(),
>> + },
>
> Actually, would be nice to handle this just like chipset for consistency, i.e.
>
> revision: boot0.revision()
Good idea, done.
...
>> + fn try_from(boot42: regs::NV_PMC_BOOT_42) -> Result<Self> {
>> + Ok(Self {
>> + chipset: boot42.chipset()?,
>> + revision: Revision {
>> + major: boot42.major_revision(),
>> + minor: boot42.minor_revision(),
>> + },
>
> Same here, could be
>
> revision: boot42.revision()
Done.
...> Without the comments this currently is:
>
> let boot42 = if boot0.is_next_gen() {
> Some(regs::NV_PMC_BOOT_42::read(bar))
> } else {
> None
> };
>
> if boot0.is_nv04() {
> Err(ENODEV)?
> }
>
> boot42
> .map(Spec::try_from)
> .unwrap_or_else(|| Spec::try_from(boot0))
>
> Which I think is a bit heavy-handed. Let's simplify this a bit:
>
> let boot0 = regs::NV_PMC_BOOT_0::read(bar);
>
> if boot0.is_nv04() {
> return Err(ENODEV);
> }
>
> Spec::try_from(
> if boot0.is_next_gen() {
> regs::NV_PMC_BOOT_42::read(bar)
> } else {
> boot0
> }
> )
That, combined with Timur's comment, made me realize that .is_next_gen()
can be made reliable enough that we don't even need is_nv04() at all.
I've shortened and fixed it up accordingly.
(I think I got a little too involved in the Nouveau-related discussions,
there: Nouveau has a different set of things to support, and to check
for.)
thanks,
--
John Hubbard
