Re: [PATCH v3 3/6] hwmon: (pmbus/core) add wp module param

[Guenter Roeck]

On 11/4/24 06:39, Jerome Brunet wrote:
> On Mon 04 Nov 2024 at 06:18, Guenter Roeck <linux@xxxxxxxxxxxx> wrote:
>
> > On 11/4/24 00:43, Jerome Brunet wrote:
> >
> > > > > +/*
> > > > > + * PMBus write protect forced mode:
> > > > > + * PMBus may come up with a variety of write protection configuration.
> > > > > + * 'pmbus_wp' may be used if a particular write protection is necessary.
> > > > > + * The ability to actually alter the protection may also depend on the chip
> > > > > + * so the actual runtime write protection configuration may differ from
> > > > > + * the requested one. pmbus_core currently support the following value:
> > > > > + * - 0: write protection removed
> > > > > + * - 1: write protection fully enabled, including OPERATION and VOUT_COMMAND
> > > > > + *      registers. Chips essentially become read-only with this.
> > > >
> > > > Would it be desirable to also suppport the ability to set the output voltage
> > > > but not limits (PB_WP_VOUT) ?
> > > I was starting simple, it is doable sure.
> > > It is not something I will be able to test on actual since does not
> > > support that.
> > > Do you want me to add "2: write protection enable execpt for
> > > VOUT_COMMAND." ?
> >
> > Please add it. I have a number of PMBus test boards and will be able to test it.
> >
> > Thee are three options, though. Per specification:
>
> Any preference for the value mapped to each mode ? Using the one from
> the spec does not seem practical (32768, 16384, 8192).
>
> The bit number, maybe (7, 6, 5) ?
>
> or just by order strongest locking ?
>
> > 1000 0000 Disable all writes except to the WRITE_PROTECT command
>
> 3
>
> > 0100 0000 Disable all writes except to the WRITE_PROTECT, OPERATION and
> >            PAGE commands
>
> 2
>
> > 0010 0000 Disable all writes except to the WRITE_PROTECT, OPERATION,
> >            PAGE, ON_OFF_CONFIG and VOUT_COMMAND commands
>
> 1 ?

Bit number does not make sense since those are just commands which happen
to use individual bits. Also, module parameters should as much as possible
be abstract and not reflect HW 1:1. Strongest locking as you suggested as
second option makes more sense, since 0 means "no locking".

Thanks,
Guenter