On Thu, Nov 21, 2024 at 03:42:34PM +0100, Kory Maincent wrote:
> From: Kory Maincent (Dent Project) <kory.maincent@xxxxxxxxxxx>
>
> Expand PSE callbacks to support the newly introduced
> pi_get/set_current_limit() and pi_get_voltage() functions. These callbacks
> allow for power limit configuration in the TPS23881 controller.
>
> Additionally, the patch includes the detected class, the current power
> delivered and the power limit ranges in the status returned, providing more
> comprehensive PoE status reporting.
>
> Signed-off-by: Kory Maincent <kory.maincent@xxxxxxxxxxx>
> ---
>
> Change in v3:
> - NIT change, use tps23881_set_val helper to set the power limit
> register value.
> - Add policy varaible internally to being able to reconfigure it after a
> PWOFF call.
>
> Change in v2:
> - Use newly introduced helpers.
> ---
> drivers/net/pse-pd/tps23881.c | 326 +++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 324 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/pse-pd/tps23881.c b/drivers/net/pse-pd/tps23881.c
> index 58864b7d28d2..b25561f95774 100644
> --- a/drivers/net/pse-pd/tps23881.c
> +++ b/drivers/net/pse-pd/tps23881.c
> @@ -25,20 +25,34 @@
> #define TPS23881_REG_GEN_MASK 0x17
> #define TPS23881_REG_NBITACC BIT(5)
> #define TPS23881_REG_PW_EN 0x19
> +#define TPS23881_REG_2PAIR_POL1 0x1e
> #define TPS23881_REG_PORT_MAP 0x26
> #define TPS23881_REG_PORT_POWER 0x29
> -#define TPS23881_REG_POEPLUS 0x40
> +#define TPS23881_REG_4PAIR_POL1 0x2a
> +#define TPS23881_REG_INPUT_V 0x2e
> +#define TPS23881_REG_CHAN1_A 0x30
> +#define TPS23881_REG_CHAN1_V 0x32
> +#define TPS23881_REG_FOLDBACK 0x40
> #define TPS23881_REG_TPON BIT(0)
> #define TPS23881_REG_FWREV 0x41
> #define TPS23881_REG_DEVID 0x43
> #define TPS23881_REG_DEVID_MASK 0xF0
> #define TPS23881_DEVICE_ID 0x02
> +#define TPS23881_REG_CHAN1_CLASS 0x4c
> #define TPS23881_REG_SRAM_CTRL 0x60
> #define TPS23881_REG_SRAM_DATA 0x61
>
> +#define TPS23881_UV_STEP 3662
> +#define TPS23881_MAX_UV 60000000
> +#define TPS23881_NA_STEP 70190
> +#define TPS23881_MAX_UA 1150000
> +#define TPS23881_MW_STEP 500
> +#define TPS23881_MIN_PI_PW_LIMIT 2000
please add units annotations to define names or comments.
> +
> struct tps23881_port_desc {
> u8 chan[2];
> bool is_4p;
> + int pw_pol;
> };
>
> struct tps23881_priv {
> @@ -102,6 +116,42 @@ static u16 tps23881_set_val(u16 reg_val, u8 chan, u8 field_offset,
> return reg_val;
> }
>
> +static int
> +tps23881_pi_set_2p_pw_limit(struct tps23881_priv *priv, u8 chan, u8 pw_pol)
> +{
> + struct i2c_client *client = priv->client;
> + int ret, reg;
> + u16 val;
> +
> + reg = TPS23881_REG_2PAIR_POL1 + (chan % 4);
> + ret = i2c_smbus_read_word_data(client, reg);
> + if (ret < 0)
> + return ret;
> +
> + val = tps23881_set_val(ret, chan, 0, 0xff, pw_pol);
> + return i2c_smbus_write_word_data(client, reg, val);
> +}
> +
> +static int
> +tps23881_pi_set_4p_pw_limit(struct tps23881_priv *priv, u8 chan, u8 pw_pol)
> +{
> + struct i2c_client *client = priv->client;
> + int ret, reg;
> + u16 val;
> +
> + if ((chan % 4) < 2)
> + reg = TPS23881_REG_4PAIR_POL1;
> + else
> + reg = TPS23881_REG_4PAIR_POL1 + 1;
tps23881_pi_set_4p_pw_limit and tps23881_pi_set_2p_pw_limit are
identical except of register offset calculation. Can you please move the
common code to a separate function and add comments on why it needs
different offset calculations.
> + ret = i2c_smbus_read_word_data(client, reg);
> + if (ret < 0)
> + return ret;
> +
> + val = tps23881_set_val(ret, chan, 0, 0xff, pw_pol);
> + return i2c_smbus_write_word_data(client, reg, val);
> +}
> +
> static int tps23881_pi_enable(struct pse_controller_dev *pcdev, int id)
> {
> struct tps23881_priv *priv = to_tps23881_priv(pcdev);
> @@ -184,7 +234,38 @@ static int tps23881_pi_disable(struct pse_controller_dev *pcdev, int id)
> BIT(chan % 4));
> }
>
> - return i2c_smbus_write_word_data(client, TPS23881_REG_DET_CLA_EN, val);
> + ret = i2c_smbus_write_word_data(client, TPS23881_REG_DET_CLA_EN, val);
> + if (ret)
> + return ret;
> +
> + /* No power policy set */
> + if (priv->port[id].pw_pol < 0)
> + return 0;
> +
> + chan = priv->port[id].chan[0];
> + ret = i2c_smbus_read_byte_data(client, TPS23881_REG_FOLDBACK);
> + if (ret < 0)
> + return ret;
> +
> + /* No need to test if the chan is PoE4 as setting either bit for a
> + * 4P configured port disables the automatic configuration on both
> + * channels.
> + */
chan re-assignment can be moved here, it is easier to understand
together with this comment.
> + chan = priv->port[id].chan[0];
> + val = tps23881_set_val(ret, chan, 0, BIT(chan % 4), BIT(chan % 4));
> + ret = i2c_smbus_write_byte_data(client, TPS23881_REG_FOLDBACK, val);
> + if (ret)
> + return ret;
> +
> + /* Set power policy */
> + if (priv->port[id].is_4p)
> + /* One chan is enough to configure the PI power limit */
> + ret = tps23881_pi_set_4p_pw_limit(priv, chan,
> + priv->port[id].pw_pol);
> + else
> + ret = tps23881_pi_set_2p_pw_limit(priv, chan,
> + priv->port[id].pw_pol);
> +
> + return ret;
> }
>
> static int tps23881_pi_is_enabled(struct pse_controller_dev *pcdev, int id)
> @@ -214,6 +295,141 @@ static int tps23881_pi_is_enabled(struct pse_controller_dev *pcdev, int id)
> return enabled;
> }
>
> +static int tps23881_pi_get_voltage(struct pse_controller_dev *pcdev, int id)
> +{
> + struct tps23881_priv *priv = to_tps23881_priv(pcdev);
> + struct i2c_client *client = priv->client;
> + int ret;
> + u64 uV;
> +
> + ret = i2c_smbus_read_word_data(client, TPS23881_REG_INPUT_V);
> + if (ret < 0)
> + return ret;
> +
> + uV = ret & 0x3fff;
> + uV *= TPS23881_UV_STEP;
> + if (uV > TPS23881_MAX_UV) {
used 0x3fff * TPS23881_UV_STEP not exceed TPS23881_MAX_UV. This sanity
check can be removed.
> + dev_err(&client->dev, "voltage read out of range\n");
> + return -ERANGE;
> + }
> +
> + return (int)uV;
> +}
> +
> +static int
> +tps23881_pi_get_chan_current(struct tps23881_priv *priv, u8 chan)
> +{
> + struct i2c_client *client = priv->client;
> + int reg, ret;
> + u64 tmp_64;
> +
> + if (chan < 4)
> + /* Registers 0x30 0x34 0x38 0x3c */
> + reg = TPS23881_REG_CHAN1_A + chan * 4;
> + else
> + /* Registers 0x31 0x35 0x39 0x3d */
> + reg = TPS23881_REG_CHAN1_A + 1 + (chan % 4) * 4;
Hm, may be:
reg = TPS23881_REG_CHAN1_A + (chan % 4) * 4 + (chan >= 4);
> +
> + ret = i2c_smbus_read_word_data(client, reg);
> + if (ret < 0)
> + return ret;
> +
> + tmp_64 = ret;
Here is missing the 0x3fff mask or FIELD_GET, same for get_voltage.
> + tmp_64 *= TPS23881_NA_STEP;
> + /* uA = nA / 1000 */
> + tmp_64 = DIV_ROUND_CLOSEST_ULL(tmp_64, 1000);
This sanity check can be removed.
> + if (tmp_64 > TPS23881_MAX_UA) {
> + dev_err(&client->dev, "current read out of range\n");
> + return -ERANGE;
> + }
> + return (int)tmp_64;
> +}
> +
> +static int
> +tps23881_pi_get_power(struct tps23881_priv *priv, unsigned long id)
> +{
> + int ret, uV, uA;
> + u64 tmp_64;
> + u8 chan;
> +
> + ret = tps23881_pi_get_voltage(&priv->pcdev, id);
> + if (ret < 0)
> + return ret;
> + uV = ret;
> +
> + chan = priv->port[id].chan[0];
> + ret = tps23881_pi_get_chan_current(priv, chan);
> + if (ret < 0)
> + return ret;
> + uA = ret;
> +
> + if (priv->port[id].is_4p) {
> + chan = priv->port[id].chan[1];
> + ret = tps23881_pi_get_chan_current(priv, chan);
> + if (ret < 0)
> + return ret;
> + uA += ret;
> + }
> +
> + tmp_64 = uV;
> + tmp_64 *= uA;
> + /* mW = uV * uA / 1000000000 */
> + return DIV_ROUND_CLOSEST_ULL(tmp_64, 1000000000);
> +}
> +
> +static int
> +tps23881_pi_get_pw_limit_chan(struct tps23881_priv *priv, u8 chan)
> +{
> + struct i2c_client *client = priv->client;
> + int ret, reg;
> + u16 val;
> +
> + reg = TPS23881_REG_2PAIR_POL1 + (chan % 4);
> + ret = i2c_smbus_read_word_data(client, reg);
> + if (ret < 0)
> + return ret;
> +
> + val = tps23881_calc_val(ret, chan, 0, 0xff);
> + return val * TPS23881_MW_STEP;
> +}
> +
> +static int tps23881_pi_get_pw_limit(struct tps23881_priv *priv, int id)
> +{
> + int ret, mW;
> + u8 chan;
> +
> + chan = priv->port[id].chan[0];
> + ret = tps23881_pi_get_pw_limit_chan(priv, chan);
> + if (ret < 0)
> + return ret;
> +
> + mW = ret;
> + if (priv->port[id].is_4p) {
> + chan = priv->port[id].chan[1];
> + ret = tps23881_pi_get_pw_limit_chan(priv, chan);
> + if (ret < 0)
> + return ret;
> + mW += ret;
> + }
> +
> + return mW;
> +}
> +
> +static int tps23881_pi_get_class(struct tps23881_priv *priv, int id)
> +{
> + struct i2c_client *client = priv->client;
> + int ret, reg;
> + u8 chan;
> +
> + chan = priv->port[id].chan[0];
> + reg = TPS23881_REG_CHAN1_CLASS + (chan % 4);
> + ret = i2c_smbus_read_word_data(client, reg);
> + if (ret < 0)
> + return ret;
> +
> + return tps23881_calc_val(ret, chan, 4, 0x0f);
> +}
> +
> static int tps23881_ethtool_get_status(struct pse_controller_dev *pcdev,
> unsigned long id,
> struct netlink_ext_ack *extack,
> @@ -256,6 +472,31 @@ static int tps23881_ethtool_get_status(struct pse_controller_dev *pcdev,
> else
> status->c33_admin_state = ETHTOOL_C33_PSE_ADMIN_STATE_DISABLED;
>
> + ret = tps23881_pi_get_power(priv, id);
> + if (ret < 0)
> + return ret;
> + status->c33_actual_pw = ret;
> +
> + status->c33_pw_limit_ranges = kzalloc(sizeof(*status->c33_pw_limit_ranges),
> + GFP_KERNEL);
> + if (!status->c33_pw_limit_ranges)
> + return -ENOMEM;
> +
> + status->c33_actual_pw = ret;
> + status->c33_pw_limit_nb_ranges = 1;
> + status->c33_pw_limit_ranges->min = TPS23881_MIN_PI_PW_LIMIT;
> + status->c33_pw_limit_ranges->max = MAX_PI_PW;
> +
> + ret = tps23881_pi_get_pw_limit(priv, id);
> + if (ret < 0)
> + return ret;
> + status->c33_avail_pw_limit = ret;
> +
> + ret = tps23881_pi_get_class(priv, id);
> + if (ret < 0)
> + return ret;
> + status->c33_pw_class = ret;
> +
> return 0;
> }
>
> @@ -553,6 +794,9 @@ tps23881_write_port_matrix(struct tps23881_priv *priv,
> if (port_matrix[i].exist)
> priv->port[pi_id].chan[0] = lgcl_chan;
>
> + /* Initialize power policy internal value */
> + priv->port[pi_id].pw_pol = -1;
> +
> /* Set hardware port matrix for all ports */
> val |= hw_chan << (lgcl_chan * 2);
>
> @@ -672,12 +916,90 @@ static int tps23881_setup_pi_matrix(struct pse_controller_dev *pcdev)
> return ret;
> }
>
> +static int tps23881_pi_get_current_limit(struct pse_controller_dev *pcdev,
> + int id)
> +{
> + struct tps23881_priv *priv = to_tps23881_priv(pcdev);
> + int ret, mW, uV;
> + u64 tmp_64;
> +
> + ret = tps23881_pi_get_pw_limit(priv, id);
> + if (ret < 0)
> + return ret;
> + mW = ret;
> +
> + ret = tps23881_pi_get_voltage(pcdev, id);
> + if (ret < 0)
> + return ret;
> + uV = ret;
> +
> + tmp_64 = mW;
> + tmp_64 *= 1000000000ull;
> + /* uA = mW * 1000000000 / uV */
> + return DIV_ROUND_CLOSEST_ULL(tmp_64, uV);
> +}
> +
> +static int tps23881_pi_set_current_limit(struct pse_controller_dev *pcdev,
> + int id, int max_uA)
> +{
> + struct tps23881_priv *priv = to_tps23881_priv(pcdev);
> + u8 chan, pw_pol;
> + int ret, mW;
> + u64 tmp_64;
> + u16 val;
> +
> + ret = tps23881_pi_get_voltage(pcdev, id);
> + if (ret < 0)
> + return ret;
> +
> + tmp_64 = ret;
> + tmp_64 *= max_uA;
> + /* mW = uV * uA / 1000000000 */
> + mW = DIV_ROUND_CLOSEST_ULL(tmp_64, 1000000000);
> + if (mW < TPS23881_MIN_PI_PW_LIMIT || MAX_PI_PW < mW)
May be add here some error message to let understand what limit did we
hit.
> + return -ERANGE;
> +
> + chan = priv->port[id].chan[0];
> + ret = i2c_smbus_read_byte_data(priv->client, TPS23881_REG_FOLDBACK);
> + if (ret < 0)
> + return ret;
I have seen this sequence in tps23881_pi_disable(), may be share this
code?
> + /* No need to test if the chan is PoE4 as setting either bit for a
> + * 4P configured port disables the automatic configuration on both
> + * channels.
> + */
> + val = tps23881_set_val(ret, chan, 0, BIT(chan % 4), BIT(chan % 4));
> + ret = i2c_smbus_write_byte_data(priv->client, TPS23881_REG_FOLDBACK, val);
> + if (ret)
> + return ret;
> +
> + pw_pol = DIV_ROUND_CLOSEST_ULL(mW, TPS23881_MW_STEP);
> +
> + /* Save power policy to reconfigure it after a disabled call */
> + priv->port[id].pw_pol = pw_pol;
> + if (priv->port[id].is_4p) {
> + /* One chan is enough to configure the PI power limit */
> + ret = tps23881_pi_set_4p_pw_limit(priv, chan, pw_pol);
> + if (ret < 0)
> + return ret;
> + } else {
> + ret = tps23881_pi_set_2p_pw_limit(priv, chan, pw_pol);
> + if (ret < 0)
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> static const struct pse_controller_ops tps23881_ops = {
> .setup_pi_matrix = tps23881_setup_pi_matrix,
> .pi_enable = tps23881_pi_enable,
> .pi_disable = tps23881_pi_disable,
> .pi_is_enabled = tps23881_pi_is_enabled,
> .ethtool_get_status = tps23881_ethtool_get_status,
> + .pi_get_voltage = tps23881_pi_get_voltage,
> + .pi_get_current_limit = tps23881_pi_get_current_limit,
> + .pi_set_current_limit = tps23881_pi_set_current_limit,
> };
>
> static const char fw_parity_name[] = "ti/tps23881/tps23881-parity-14.bin";
>
> --
> 2.34.1
>
>
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 |
