On 2022-02-15 09:37, Christian König wrote: > > > Am 15.02.22 um 15:33 schrieb Luben Tuikov: >> On 2022-02-15 09:25, Christian König wrote: >>> Am 15.02.22 um 00:21 schrieb Luben Tuikov: >>>> Add the "harvest" field to the IP attributes in >>>> the IP discovery sysfs visualization, as this >>>> field is present in the binary data. >>>> >>>> Cc: Alex Deucher <Alexander.Deucher@xxxxxxx> >>>> Signed-off-by: Luben Tuikov <luben.tuikov@xxxxxxx> >>>> --- >>>> drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c | 8 ++++++++ >>>> 1 file changed, 8 insertions(+) >>>> >>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c >>>> index c8dbdb78988ce0..0496d369504641 100644 >>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c >>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c >>>> @@ -393,6 +393,7 @@ struct ip_hw_instance { >>>> int hw_id; >>>> u8 num_instance; >>>> u8 major, minor, revision; >>>> + u8 harvest; >>> Should we maybe use bool here instead? >>> >>> Apart from that looks good to me. >> Thanks Christian. >> >> I don't mind using bool here. >> >> I saw the field in the binary data is 4 bits and represented that. >> >> Is the field actually bool in the IP binary data? > > I'm not sure either. > > I would have expected it to be a single bit flag in the binary, but > 4bits sounds like it serves some more purpose. > > Probably best to stick to u8 for now in this case. Okay. Can I get a R-B then? Regards, Luben > > Regards, > Christian. > >> I can change the patch and resubmit. >> >> Regards, >> Luben >> >>> Regards, >>> Christian. >>> >>>> >>>> int num_base_addresses; >>>> u32 base_addr[]; >>>> @@ -440,6 +441,11 @@ static ssize_t revision_show(struct ip_hw_instance *ip_hw_instance, char *buf) >>>> return sysfs_emit(buf, "%d\n", ip_hw_instance->revision); >>>> } >>>> >>>> +static ssize_t harvest_show(struct ip_hw_instance *ip_hw_instance, char *buf) >>>> +{ >>>> + return sysfs_emit(buf, "0x%01X\n", ip_hw_instance->harvest); >>>> +} >>>> + >>>> static ssize_t num_base_addresses_show(struct ip_hw_instance *ip_hw_instance, char *buf) >>>> { >>>> return sysfs_emit(buf, "%d\n", ip_hw_instance->num_base_addresses); >>>> @@ -471,6 +477,7 @@ static struct ip_hw_instance_attr ip_hw_attr[] = { >>>> __ATTR_RO(major), >>>> __ATTR_RO(minor), >>>> __ATTR_RO(revision), >>>> + __ATTR_RO(harvest), >>>> __ATTR_RO(num_base_addresses), >>>> __ATTR_RO(base_addr), >>>> }; >>>> @@ -708,6 +715,7 @@ static int amdgpu_discovery_sysfs_ips(struct amdgpu_device *adev, >>>> ip_hw_instance->major = ip->major; >>>> ip_hw_instance->minor = ip->minor; >>>> ip_hw_instance->revision = ip->revision; >>>> + ip_hw_instance->harvest = ip->harvest; >>>> ip_hw_instance->num_base_addresses = ip->num_base_address; >>>> >>>> for (kk = 0; kk < ip_hw_instance->num_base_addresses; kk++) >>>> >>>> base-commit: d8604f1d237a145db48bae4ea60b85a5875df307 >> Regards, > Regards, -- Luben