[PATCH 3/3] cmd: dm: Fixed/Added DM driver listing subcommands

[Niel Fourie]

Hi Sean, Simon,

I will write tests for all of these cases as Simon requested. Thanks, it 
did not occur to me.

On 3/17/20 7:51 PM, Sean Anderson wrote:
>> -void dm_dump_drivers(void)
>> +void dm_dump_driver_compat(void)
>>   {
>>   	struct driver *d = ll_entry_start(struct driver, driver);
>>   	const int n_ents = ll_entry_count(struct driver, driver);
>> @@ -107,6 +107,9 @@ void dm_dump_drivers(void)
>>   	puts("Driver                Compatible\n");
>>   	puts("--------------------------------\n");
>>   	for (entry = d; entry < d + n_ents; entry++) {
>> +		if (!entry->of_match) {
>> +			continue;
>> +		}
> 
> This should have been fixed in version 2 of the patch [1].

Ah, I didn't know of that patch. I'll ask Tom whether I should rebase my 
patches on top of that change or not.

>> +
>> +void dm_dump_drivers(void)
>> +{
>> +	struct driver *d = ll_entry_start(struct driver, driver);
>> +	const int n_ents = ll_entry_count(struct driver, driver);
>> +	struct driver *entry;
>> +	struct udevice *udev;
>> +	struct uclass *uc;
>> +	int i;
>> +
>> +	puts("Driver                    uid uclass               Devices\n");
>> +	for (i = 0; i < 77; i++)
>> +		putc('-');
> 
> Can you print these dashes in a way which makes it obvious that they are
> the correct length? E.g. do something like
> 
> puts("Driver                    uid uclass               Devices\n");
> puts("----------------------------------------------------------\n");
> 

Ah, I was lining the dashes up with the contents instead of the heading. 
I will fix all cases to be like the above. Thanks!

>> +void dm_dump_static_driver_info(void)
>> +{
>> +	struct driver_info *drv = ll_entry_start(struct driver_info,
>> +						 driver_info);
>> +	const int n_ents = ll_entry_count(struct driver_info, driver_info);
>> +	struct driver_info *entry;
>> +
>> +	puts("Driver               Address\n");
>> +	puts("------------------------------\n");
>> +	for (entry = drv; entry != drv + n_ents; entry++) {
>> +		printf("%-20.20s @%08lx\n", entry->name,
>> +		       (ulong)map_to_sysmem(entry->platdata));
>> +	}
>> +}
> 
> Just curious, what were you using this for?

In general I was looking for ways to query list-able capabilities of 
U-boot at run time, to show anomalies and to highlight legacy/non-ideal 
implementations when possible; it is hard to fix a problem which you 
can't see it.

Because the U_BOOT_DEVICE() devices should be used sparingly and do not 
appear in the device tree, I felt that it was useful to list them to see 
their impact. The address is useful to for eyeballing the platform data 
with md.b to distinguish between multiple driver instances...

In the hunt against legacy drivers, I am also looking at ways of 
listing/highlighting other problem-children at run time, but whether 
those patches will see light-of-day is another matter...

Best regards,
Niel Fourie

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-21 Fax: +49-8142-66989-80  Email: lusus at denx.de