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>


Kind regards,

Paul

Reply via email to