>From: Paul Menzel <pmen...@molgen.mpg.de>
>Sent: Wednesday, September 11, 2024 8:22 AM
>
>Dear Arkadiusz,
>
>
>Thank you for your patch. It’d be great if you made the summary more
>explicit. For example:
>
>ice: Disallow DPLL_PIN_STATE_SELECTABLE for dpll output pins
>
>Am 11.09.24 um 01:28 schrieb Arkadiusz Kubalewski:
>> Currently the user may request DPLL_PIN_STATE_SELECTABLE for an output
>> pin, and this would actually set the DISCONNECTED state instead.
>> It doesn't make any sense. SELECTABLE is valid only in case of input pins
>
>It’d be great if you added a blank line between paragraphs.
>
>> (on AUTOMATIC type dpll), where dpll itself would select best valid input.
>> For the output pin only CONNECTED/DISCONNECTED are expected.
>>
>> Fixes: d7999f5ea64b ("ice: implement dpll interface to control cgu")
>> Reviewed-by: Aleksandr Loktionov <aleksandr.loktio...@intel.com>
>> Signed-off-by: Arkadiusz Kubalewski <arkadiusz.kubalew...@intel.com>
>> ---
>> drivers/net/ethernet/intel/ice/ice_dpll.c | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/net/ethernet/intel/ice/ice_dpll.c
>b/drivers/net/ethernet/intel/ice/ice_dpll.c
>> index e92be6f130a3..3d20c3b232aa 100644
>> --- a/drivers/net/ethernet/intel/ice/ice_dpll.c
>> +++ b/drivers/net/ethernet/intel/ice/ice_dpll.c
>> @@ -651,6 +651,8 @@ ice_dpll_output_state_set(const struct dpll_pin *pin,
>void *pin_priv,
>> struct ice_dpll_pin *p = pin_priv;
>> struct ice_dpll *d = dpll_priv;
>>
>> + if (state == DPLL_PIN_STATE_SELECTABLE)
>> + return -EINVAL;
>> if (!enable && p->state[d->dpll_idx] == DPLL_PIN_STATE_DISCONNECTED)
>> return 0;
>>
>
>With the above addressed:
>
>Reviewed-by: Paul Menzel <pmen...@molgen.mpg.de>
>
Greetings Paul,
Sure, fixed both as suggested in v2.
Thank you for your review!
Arkadiusz
>
>Kind regards,
>
>Paul
