>-----Original Message----- >From: Mikko Perttunen [mailto:cyndis@xxxxxxxx] >Sent: Wednesday, December 21, 2016 3:01 PM >To: Preetham Chandru; Mikko Perttunen; 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 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) > Ok, sounds good to me. Will move it to soc_data structure. >Thanks, >Mikko. ��.n��������+%������w��{.n�����{��נ���^n�r������&��z�ޗ�zf���h���~����������_��+v���)ߣ�
