Received-SPF: softfail (google.com: domain of transitioning linux-kernel-owner@vger.kernel.org does not designate 23.128.96.19 as permitted sender) client-ip=23.128.96.19;
Message-ID: <7ab071a2-4e5e-9aea-6887-d3e4281de607@redhat.com>
Date:   Fri, 25 Feb 2022 22:19:15 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.4.0
Subject: Re: [PATCH v2 1/3] ata: ahci: Rename board_ahci_mobile
Content-Language: en-US
To:     "Limonciello, Mario" <Mario.Limonciello@amd.com>,
        Christoph Hellwig <hch@infradead.org>
Cc:     Damien Le Moal <damien.lemoal@opensource.wdc.com>,
        "open list:LIBATA SUBSYSTEM (Serial and Parallel ATA drivers)" 
        <linux-ide@vger.kernel.org>,
        open list <linux-kernel@vger.kernel.org>,
        "pmenzel@molgen.mpg.de" <pmenzel@molgen.mpg.de>
References: <20220225061113.223920-1-mario.limonciello@amd.com>
 <Yhj9Pdp/sHASmBw4@infradead.org>
 <BL1PR12MB5157D29423AE95EE32F00303E23E9@BL1PR12MB5157.namprd12.prod.outlook.com>
 <e65c4fbb-95d0-5c5a-2b15-414b519d3319@redhat.com>
 <BL1PR12MB5157D6984E5855701A9449E0E23E9@BL1PR12MB5157.namprd12.prod.outlook.com>
 <ee9a51e2-1733-dcd5-7514-0b8d1c1fa430@redhat.com>
 <BL1PR12MB5157B104CF62D747587A0412E23E9@BL1PR12MB5157.namprd12.prod.outlook.com>
From:   Hans de Goede <hdegoede@redhat.com>
In-Reply-To: <BL1PR12MB5157B104CF62D747587A0412E23E9@BL1PR12MB5157.namprd12.prod.outlook.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

Hi,

On 2/25/22 22:13, Limonciello, Mario wrote:
> [AMD Official Use Only]
> 
>>>>>> On Fri, Feb 25, 2022 at 12:11:11AM -0600, Mario Limonciello wrote:
>>>>>>> This board definition was originally created for mobile devices to
>>>>>>> designate default link power managmeent policy to influence runtime
>>>>>>> power consumption.
>>>>>>>
>>>>>>> As this is interesting for more than just mobile designs, rename the
>>>>>>> board to `board_ahci_low_power` to make it clear it is about default
>>>>>>> policy.
>>>>>>
>>>>>> Is there any good reason to not just apply the policy to all devices
>>>>>> by default?
>>>>>
>>>>> That sure would make this all cleaner.
>>>>>
>>>>> I think Hans knows more of the history here than anyone else.  I had
>>>>> presumed there was some data loss scenarios with some of the older
>>>>> chipsets.
>>>>
>>>> When I first introduced this change there were reports of crashes and
>>>> data corruption caused by setting the policy to min_power, these were
>>>> tied to some motherboards and/or to some drives.
>>>>
>>>> This is the whole reason why I only enabled this on a subset of all the
>>>> AHCI chipsets.
>>>>
>>>> At least on devices with a chipset which is currently marked as
>>>> mobile, the motherboard specific issues could be fixed with a BIOS
>>>> update. But I doubt that similar BIOS fixes have also been rolled
>>>> out to all desktop boards (and have been applied by all users),
>>>> and I also don't know about older boards.
>>>>
>>>> So enabling this on all chipsets is definitely not without risks.
>>>>
>>>
>>> This was before min_power_with_partial and min_power_with_dipm
>>> were introduced though right?
>>
>> The issues where some laptops needed BIOS updates was with fedora
>> using min_power_with_dipm as default for mobile chipsets.
>>
> 
> Do you know if the drives actually supported slumber and partial? 
> I wonder if that was the real problem that they were being set when
> they shouldn't be.> 
> I added something for this in 2/2 in the RFC series you can look at.

Fedora defaults to ATA_LPM_MED_POWER_WITH_DIPM so patch 2/2 is
a no-op on Fedora; and IIRC (it has been a long time) the
need for BIOS updates on some mobile devices was with
standard Fedora kernels / settings.

Regards,

Hans


> 
>>>  Maybe another way to look at this
>>> is to drop the policy min_power, which overall is dangerous.
>>
>> Maybe, see above. I'm not going to block this if people want
>> to give this a try, but it is going to require someone keeping
>> a very close look at any issues popping up and we must be
>> prepared to roll-back the change if necessary.
>>
> 
> Per Paul's suggestion I sent out v3 of this series and then I sent
> out a separate RFC series (you're on CC).  For this type of
> thing if y'all think it makes sense to do.