On Fri, Aug 19, 2022 at 10:54:14PM +0200, Andrew Lunn wrote:
> > +static int
> > +gen_pse_podl_get_admin_sate(struct pse_controller_dev *pcdev, unsigned long id)
>
> Should that be state?
ack. fixed.
> > +{
> > + struct gen_pse_priv *priv = to_gen_pse(pcdev);
> > +
> > + /* aPoDLPSEAdminState can be different to aPoDLPSEPowerDetectionStatus
> > + * which is provided by the regulator.
> > + */
> > + return priv->admin_state;
> > +}
> > +
> > +static int
> > +gen_pse_podl_set_admin_control(struct pse_controller_dev *pcdev,
> > + unsigned long id,
> > + enum ethtool_podl_pse_admin_state state)
> > +{
> > + struct gen_pse_priv *priv = to_gen_pse(pcdev);
> > + int ret;
> > +
> > + if (priv->admin_state == state)
> > + goto set_state;
>
> return 0; ?
ack. done
> > + platform_set_drvdata(pdev, priv);
> > +
> > + priv->admin_state = ETHTOOL_PODL_PSE_ADMIN_STATE_DISABLED;
>
> There is the comment earlier:
>
> /* aPoDLPSEAdminState can be different to aPoDLPSEPowerDetectionStatus
> * which is provided by the regulator.
>
> Is this because the regulator might of been turned on, but it has
> detected a short and turned itself off? So it is administratively on,
> but off in order to stop the magic smoke escaping?
ack. According to 30.15.1.1.3 aPoDLPSEPowerDetectionStatus, we may have
following:
/**
* enum ethtool_podl_pse_pw_d_status - power detection status of the PoDL PSE.
* IEEE 802.3-2018 30.15.1.1.3 aPoDLPSEPowerDetectionStatus:
* @ETHTOOL_PODL_PSE_PW_D_STATUS_UNKNOWN: PoDL PSE
* @ETHTOOL_PODL_PSE_PW_D_STATUS_DISABLED: "The enumeration “disabled” is
* asserted true when the PoDL PSE state diagram variable mr_pse_enable is
* false"
* @ETHTOOL_PODL_PSE_PW_D_STATUS_SEARCHING: "The enumeration “searching” is
* asserted true when either of the PSE state diagram variables
* pi_detecting or pi_classifying is true."
* @ETHTOOL_PODL_PSE_PW_D_STATUS_DELIVERING: "The enumeration “deliveringPower”
* is asserted true when the PoDL PSE state diagram variable pi_powered is
* true."
* @ETHTOOL_PODL_PSE_PW_D_STATUS_SLEEP: "The enumeration “sleep” is asserted
* true when the PoDL PSE state diagram variable pi_sleeping is true."
* @ETHTOOL_PODL_PSE_PW_D_STATUS_IDLE: "The enumeration “idle” is asserted true
* when the logical combination of the PoDL PSE state diagram variables
* pi_prebiased*!pi_sleeping is true."
* @ETHTOOL_PODL_PSE_PW_D_STATUS_ERROR: "The enumeration “error” is asserted
* true when the PoDL PSE state diagram variable overload_held is true."
*/
Probably all of them, except of ETHTOOL_PODL_PSE_PW_D_STATUS_SEARCHING can be
potentially implemented in this driver on top of regulator framework.
> But what about the other way around? Something has already turned the
> regulator on, e.g. the bootloader. Should the default be
> ETHTOOL_PODL_PSE_ADMIN_STATE_DISABLED even thought power is being
> delivered? Do we want to put the regulator into the off state at
> probe, so it is in a well defined state? Or set priv->admin_state to
> whatever regulator_is_enabled() indicates?
Good question. I assume, automotive folks would love be able to enable
regulator in the boot loader on the PSE to let the Powered Device boot parallel
to the PSE.
Regards,
Oleksij
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
