Re: [PATCH v2 2/5] tpm: Extend TPMIfClass with get_irqnum() function

[Stefan Berger]

On 6/15/20 11:54 AM, Philippe Mathieu-Daudé wrote:
> On 6/15/20 4:23 PM, Stefan Berger wrote:
> > From: Stefan Berger <stefanb@sbct-2.pok.ibm.com>
> >
> > Implement get_irqnum() as part of the TPMIfClass to be get the assigned
> > IRQ number.
> >
> > Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
> > ---
> >   hw/tpm/tpm_tis_isa.c    |  9 +++++++++
> >   hw/tpm/tpm_tis_sysbus.c |  9 +++++++++
> >   include/sysemu/tpm.h    | 10 ++++++++++
> >   3 files changed, 28 insertions(+)
> >
> > diff --git a/hw/tpm/tpm_tis_isa.c b/hw/tpm/tpm_tis_isa.c
> > index 30ba37079d..63b62f4c21 100644
> > --- a/hw/tpm/tpm_tis_isa.c
> > +++ b/hw/tpm/tpm_tis_isa.c
> > @@ -80,6 +80,14 @@ static enum TPMVersion tpm_tis_isa_get_tpm_version(TPMIf *ti)
> >       return tpm_tis_get_tpm_version(s);
> >   }
> >   
> > +static uint8_t tpm_tis_isa_get_irqnum(TPMIf *ti)
> > +{
> > +    TPMStateISA *isadev = TPM_TIS_ISA(ti);
> > +    TPMState *s = &isadev->state;
> > +
> > +    return s->irq_num;
> > +}
> > +
> >   static void tpm_tis_isa_reset(DeviceState *dev)
> >   {
> >       TPMStateISA *isadev = TPM_TIS_ISA(dev);
> > @@ -148,6 +156,7 @@ static void tpm_tis_isa_class_init(ObjectClass *klass, void 
> > *data)
> >       dc->reset = tpm_tis_isa_reset;
> >       tc->request_completed = tpm_tis_isa_request_completed;
> >       tc->get_version = tpm_tis_isa_get_tpm_version;
> > +    tc->get_irqnum = tpm_tis_isa_get_irqnum;
> >   }
> >   
> >   static const TypeInfo tpm_tis_isa_info = {
> > diff --git a/hw/tpm/tpm_tis_sysbus.c b/hw/tpm/tpm_tis_sysbus.c
> > index eced1fc843..6cf45e5057 100644
> > --- a/hw/tpm/tpm_tis_sysbus.c
> > +++ b/hw/tpm/tpm_tis_sysbus.c
> > @@ -80,6 +80,14 @@ static enum TPMVersion tpm_tis_sysbus_get_tpm_version(TPMIf 
> > *ti)
> >       return tpm_tis_get_tpm_version(s);
> >   }
> >   
> > +static uint8_t tpm_tis_sysbus_get_irqnum(TPMIf *ti)
> > +{
> > +    TPMStateSysBus *sbdev = TPM_TIS_SYSBUS(ti);
> > +    TPMState *s = &sbdev->state;
> > +
> > +    return s->irq_num;
> > +}
> > +
> >   static void tpm_tis_sysbus_reset(DeviceState *dev)
> >   {
> >       TPMStateSysBus *sbdev = TPM_TIS_SYSBUS(dev);
> > @@ -137,6 +145,7 @@ static void tpm_tis_sysbus_class_init(ObjectClass *klass, 
> > void *data)
> >       dc->reset = tpm_tis_sysbus_reset;
> >       tc->request_completed = tpm_tis_sysbus_request_completed;
> >       tc->get_version = tpm_tis_sysbus_get_tpm_version;
> > +    tc->get_irqnum = tpm_tis_sysbus_get_irqnum;
> >   }
> >   
> >   static const TypeInfo tpm_tis_sysbus_info = {
> > diff --git a/include/sysemu/tpm.h b/include/sysemu/tpm.h
> > index 03fb25941c..b94a8a2b16 100644
> > --- a/include/sysemu/tpm.h
> > +++ b/include/sysemu/tpm.h
> > @@ -41,6 +41,7 @@ typedef struct TPMIfClass {
> >       enum TpmModel model;
> >       void (*request_completed)(TPMIf *obj, int ret);
> >       enum TPMVersion (*get_version)(TPMIf *obj);
> > +    uint8_t (*get_irqnum)(TPMIf *obj);
> >   } TPMIfClass;
> >   
> >   #define TYPE_TPM_TIS_ISA            "tpm-tis"
> > @@ -74,4 +75,13 @@ static inline TPMVersion tpm_get_version(TPMIf *ti)
> >       return TPM_IF_GET_CLASS(ti)->get_version(ti);
> >   }
> >   
> > +static inline uint8_t tpm_get_irqnum(TPMIf *ti)
> > +{
> > +    if (!ti || !TPM_IF_GET_CLASS(ti)->get_irqnum) {
> > +        return 0;
> 0 is valid...
>
> Shouldn't we use an Error* argument?
>
> Else return -1 when invalid?

Indeed. Let me fix this. Thanks.


> > +    }
> > +
> > +    return TPM_IF_GET_CLASS(ti)->get_irqnum(ti);
> > +}
> > +
> >   #endif /* QEMU_TPM_H */