[Intel-gfx] [PATCH v3 2/3] ACPI / PMIC: Implement exec_mipi_pmic_seq_element for CHT Whiskey Cove PMIC

[Hans de Goede]

Hi,

On 13-12-18 14:08, Ville Syrjälä wrote:
> On Thu, Dec 13, 2018 at 01:40:27PM +0100, Hans de Goede wrote:
>> Hi,
>>
>> On 13-12-18 13:14, Ville Syrjälä wrote:

<snip>

>>>> +static int intel_cht_wc_exec_mipi_pmic_seq_element(struct regmap *regmap,
>>>> +						   const u8 *data)
>>>> +{
>>>> +	u32 value, mask, reg_address, address;
>>>> +	u16 i2c_client_address;
>>>> +
>>>> +	/* byte 0 aka PMIC Flag is reserved */
>>>> +	i2c_client_address	= get_unaligned_le16(data + 1);
>>>> +	reg_address		= get_unaligned_le32(data + 3);
>>>> +	value			= get_unaligned_le32(data + 7);
>>>> +	mask			= get_unaligned_le32(data + 11);
>>>
>>> Upon further reflection maybe it would better to do this decoding in
>>> the i915 code and just pass each parameter to this hook separately?
>>> That way we wouldn't be spreading the vbt details all over the place.
>>
>> Interesting point, if the VBT spec says that this encoding is PMIC
>> independent, then yes we should probably fo the decoding in the VBT
>> code and change the intel_soc_pmic_exec_mipi_pmic_seq_element
>> prototype to:
>>
>> int intel_soc_pmic_exec_mipi_pmic_seq_element(u16 i2c_address, u32 reg_address,
>> 					      u32 value, u32 mask);
>>
>> If you agree please let me know and I will do a v4 of the patchset.
> 
> Yeah, I think that's probably better. The spec has just the one
> interpretation for the sequence.

Ok, v4. coming up.

> Oh, and we should probably change the DRM_DEBUG_KMS() for the
> PMIC_OPREGION=n case to a DRM_ERROR() which tells people to
> enable PMIC_OPREGION=y.

Will do.

Regards,

Hans

p.s.

Have you also seen these 2 DSI related series (I've not see any feedback
on these 2 series yet) ?  :

https://patchwork.kernel.org/patch/10707629/  (patch 1/4)
https://patchwork.kernel.org/patch/10707647/  (patch 1/4)