Re: [PATCH v2 07/12] acpi: move aml builder code for rtc device

[Gerd Hoffmann]

On Mon, Apr 06, 2020 at 02:17:05PM +0200, Igor Mammedov wrote:
> On Mon, 6 Apr 2020 10:25:17 +0200
> Gerd Hoffmann <address@hidden> wrote:
> 
> > On Fri, Apr 03, 2020 at 12:09:21PM +0200, Igor Mammedov wrote:
> > > On Fri,  3 Apr 2020 10:04:57 +0200
> > > Gerd Hoffmann <address@hidden> wrote:
> > >   
> > > > Signed-off-by: Gerd Hoffmann <address@hidden>
> > > > ---  
> > > [...]  
> > > > +static void rtc_build_aml(ISADevice *isadev, Aml *scope)
> > > > +{
> > > > +    Aml *dev;
> > > > +    Aml *crs;
> > > > +
> > > > +    crs = aml_resource_template();
> > > > +    aml_append(crs, aml_io(AML_DECODE16, 0x0070, 0x0070, 0x10, 0x02));
> > > > +    aml_append(crs, aml_irq_no_flags(8));
> > > > +    aml_append(crs, aml_io(AML_DECODE16, 0x0072, 0x0072, 0x02, 0x06)); 
> > > >  
> > > 
> > > since this is made a part of device, can we fetch io port values from
> > > device instead of hard-codding values here?  
> > 
> > No, the rtc device hasn't a configurable io port address.
> what I'm after is consistent code, so if we switch to taking
> io_base/irq from ISA device, then do it everywhere.

The patch series does it consistently where it makes sense.
That IMHO isn't the case for the rtc.  It has a fixed address.
You can't have multiple instances if it.  And because of that
there isn't a variable in the device state struct where I could
read the iobase from ...

> So we don't have a zoo of devices doing the same thing in multiple
> ways.

It's two ways: hardcoded for devices which can't move and
read-from-device for devices which can move.

cheers,
  Gerd