Re: [PATCH v1 1/2] hwmon: (it87) Allow calling superio_enter outside mux

[Guenter Roeck <linux@roeck-us.net>]

On Wed, Jan 04, 2023 at 11:37:28AM +1100, Frank Crawford wrote:
> On Tue, 2023-01-03 at 10:37 -0800, Guenter Roeck wrote:
> > On Tue, Jan 03, 2023 at 05:46:11PM +1100, Frank Crawford wrote:
> > > Allow for superio_enter to be called outside mux, in particular for
> > 
> > "outside mux" is really a bad wording. I had to look into the code
> > to understand what it means. "without requesting the muxed memory
> > region", maybe.
> > 
> > Guenter
> 
> Will update with better wording, such as you suggest.
> > 
> > > initialisation of the second chipset, which must be entered, but
> > > never
> > > exited.
> > 
> > The second chipset is not "entered", it must enter configuration
> > mode (or be put into configuration mode). The name of the function
> > does not reflect the associated functionality.
> > 
> > Please rephrase.
> 
> Will do.
> 
> Do you believe the function should be renamed as well?

No. "enter" in the function name is short for "enter configuration mode".
That is good enough for a function name. Description and documentation need
to be more elaborate, though.

Guenter

> > 
> > Thanks,
> > Guenter
> 
> Regards
> Frank
> > 
> > > 
> > > Signed-off-by: Frank Crawford <frank@crawford.emu.id.au>
> > > ---
> > >  drivers/hwmon/it87.c | 13 +++++++++----
> > >  1 file changed, 9 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/drivers/hwmon/it87.c b/drivers/hwmon/it87.c
> > > index 9997f76b1f4a..4ebce2c661d7 100644
> > > --- a/drivers/hwmon/it87.c
> > > +++ b/drivers/hwmon/it87.c
> > > @@ -87,6 +87,14 @@ static struct platform_device *it87_pdev[2];
> > >  #define        DEVID   0x20    /* Register: Device ID */
> > >  #define        DEVREV  0x22    /* Register: Device Revision */
> > >  
> > > +static inline void __superio_enter(int ioreg)
> > > +{
> > > +       outb(0x87, ioreg);
> > > +       outb(0x01, ioreg);
> > > +       outb(0x55, ioreg);
> > > +       outb(ioreg == REG_4E ? 0xaa : 0x55, ioreg);
> > > +}
> > > +
> > >  static inline int superio_inb(int ioreg, int reg)
> > >  {
> > >         outb(reg, ioreg);
> > > @@ -124,10 +132,7 @@ static inline int superio_enter(int ioreg)
> > >         if (!request_muxed_region(ioreg, 2, DRVNAME))
> > >                 return -EBUSY;
> > >  
> > > -       outb(0x87, ioreg);
> > > -       outb(0x01, ioreg);
> > > -       outb(0x55, ioreg);
> > > -       outb(ioreg == REG_4E ? 0xaa : 0x55, ioreg);
> > > +       __superio_enter(ioreg);
> > >         return 0;
> > >  }
> > >  
> > > -- 
> > > 2.38.1
> > >