Re: [v2,2/3] ata: ahci_tegra: Add support to disable features

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 21.12.2016 11:12, Preetham Chandru wrote:


-----Original Message-----
From: Mikko Perttunen
Sent: Monday, November 28, 2016 6:09 PM
To: Preetham Chandru; tj@xxxxxxxxxx; swarren@xxxxxxxxxxxxx;
thierry.reding@xxxxxxxxx; preetham260@xxxxxxxxx
Cc: Laxman Dewangan; linux-ide@xxxxxxxxxxxxxxx; Venu Byravarasu; Pavan
Kunapuli; linux-tegra@xxxxxxxxxxxxxxx
Subject: Re: [v2,2/3] ata: ahci_tegra: Add support to disable features

On 24.11.2016 09:43, PREETHAM RAMACHANDRA wrote:
From: Preetham Chandru R <pchandru@xxxxxxxxxx>

Add support to disable DIPM, HIPM, DevSlp, partial, slumber and NCQ
features from DT. By default these features are enabled.


Why are these features disabled? Are they broken on all Tegra210 chips,
broken on specific boards or do they require some support from the driver
that is not there?

Most likely we should hardcode the disabled features into the driver instead
of reading them from the device tree.


Yes, currently DevSlp and DIPM features are broken for t210 and t124 but devslp is fixed for tegra186.
 Also I thought it would be nice to provide similar options for other features as well.
Since we do not support hotplug this would help us in debugging if we face issues with certain drives during kernel boot.
We can disable features we think are causing issues and retry.

We should put this data into the soc_data struct in the driver, since it is easily determined from just the compatibility string; for example

struct tegra_ahci_soc {
	...
	uint32_t quirks;
};

then

.quirks = NO_DEVSLP | NO_DIPM

or the other way around and list the features that we want to enable. This is also quite easy to change to test.

The device tree should also only contain information that describes the hardware itself, and nothing related to any specific driver's operation; so if there's a possibility that some feature is not working due to a driver issue, then the device tree definitely should not contain any entry disabling that feature.

(The downstream kernel can of course carry such features as patches on top, if required for some reason - but naturally it increases maintenance effort)

Thanks,
Mikko.
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Filesystems]     [Linux SCSI]     [Linux RAID]     [Git]     [Kernel Newbies]     [Linux Newbie]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Samba]     [Device Mapper]

  Powered by Linux