On Tue, Sep 17, 2024 at 11:47:00PM +0530, Shyam Sundar S K wrote:
> On 9/14/2024 00:38, Andy Shevchenko wrote:
> > On Fri, Sep 13, 2024 at 05:41:07PM +0530, Shyam Sundar S K wrote:
...
> >> +static void amd_asf_update_bytes(struct amd_asf_dev *dev, u8 bit, bool set)
> >
> > I didn't get the naming, the above using IO port with _bits, and this is MMIO
> > with _bytes. Are you sure the naming schema is correct?
>
> Thinking to merge both the functions into one, something like this:
>
> enum io_type {
> IO_PORT,
> MMIO
> };
>
> static void amd_asf_update_target(struct amd_asf_dev *dev, enum
> io_type type, u8 bit, bool set)
> {
>
> ...
>
> }
I'm not talking about merging them (and merged variant seems less readable
to me), but about naming. I.o.w. it's not obvious what the difference _bits vs.
_bytes.
> >> +{
> >> + unsigned long reg;
> >> +
> >> + reg = ioread32(dev->mmio_cfg.addr);
> >> + if (set)
> >> + set_bit(bit, ®);
> >> + else
> >> + clear_bit(bit, ®);
> >> + iowrite32(reg, dev->mmio_cfg.addr);
> >
> > Ditto (bitops and related things).
> >
> >> +}
--
With Best Regards,
Andy Shevchenko
