diff mbox series

iio: xilinx-ams: Add labels

Message ID 20240620204842.817237-1-sean.anderson@linux.dev (mailing list archive)
State New, archived
Headers show
Series iio: xilinx-ams: Add labels | expand

Commit Message

Sean Anderson June 20, 2024, 8:48 p.m. UTC 
Label all the channels using names from the reference manual. Some of
the "control" channels are duplicates of other channels. The reference
manual describes it like:

> The AMS register set includes several measurement registers that are
> written to by the PS SYSMON unit using the single-channel mode
> (sequencer off). These voltage measurements are performed using the
> unipolar sampling circuit with a 0 to 3V range and do not have alarms
> or minimum/maximum registers.

So I think these really are measuring the same voltages but in a
different location. In which case, sharing labels makes sense to me.

Signed-off-by: Sean Anderson <sean.anderson@linux.dev>
---

 drivers/iio/adc/xilinx-ams.c | 107 +++++++++++++++++++----------------
 1 file changed, 59 insertions(+), 48 deletions(-)

Comments

Jonathan Cameron June 23, 2024, 11:59 a.m. UTC | #1 
On Thu, 20 Jun 2024 16:48:42 -0400
Sean Anderson <sean.anderson@linux.dev> wrote:

> Label all the channels using names from the reference manual. Some of
> the "control" channels are duplicates of other channels. The reference
> manual describes it like:
> 
> > The AMS register set includes several measurement registers that are
> > written to by the PS SYSMON unit using the single-channel mode
> > (sequencer off). These voltage measurements are performed using the
> > unipolar sampling circuit with a 0 to 3V range and do not have alarms
> > or minimum/maximum registers.  
> 
> So I think these really are measuring the same voltages but in a
> different location. In which case, sharing labels makes sense to me.
> 
> Signed-off-by: Sean Anderson <sean.anderson@linux.dev>
Hi Sean,

This LGTM, but I'd like some feedback from others more familiar with the
driver and device before I apply it.  If everyone is too busy and
I don't get any replies for a few weeks I may pick it up anyway.

Feel free to poke me if I seem to have forgotten in say 2 weeks.


Jonathan

> ---
> 
>  drivers/iio/adc/xilinx-ams.c | 107 +++++++++++++++++++----------------
>  1 file changed, 59 insertions(+), 48 deletions(-)
> 
> diff --git a/drivers/iio/adc/xilinx-ams.c b/drivers/iio/adc/xilinx-ams.c
> index aa05f24931f9..f051358d6b50 100644
> --- a/drivers/iio/adc/xilinx-ams.c
> +++ b/drivers/iio/adc/xilinx-ams.c
> @@ -222,7 +222,7 @@ enum ams_ps_pl_seq {
>  #define PL_SEQ(x)		(AMS_PS_SEQ_MAX + (x))
>  #define AMS_CTRL_SEQ_BASE	(AMS_PS_SEQ_MAX * 3)
>  
> -#define AMS_CHAN_TEMP(_scan_index, _addr) { \
> +#define AMS_CHAN_TEMP(_scan_index, _addr, _name) { \
>  	.type = IIO_TEMP, \
>  	.indexed = 1, \
>  	.address = (_addr), \
> @@ -232,9 +232,10 @@ enum ams_ps_pl_seq {
>  	.event_spec = ams_temp_events, \
>  	.scan_index = _scan_index, \
>  	.num_event_specs = ARRAY_SIZE(ams_temp_events), \
> +	.datasheet_name = _name, \
>  }
>  
> -#define AMS_CHAN_VOLTAGE(_scan_index, _addr, _alarm) { \
> +#define AMS_CHAN_VOLTAGE(_scan_index, _addr, _alarm, _name) { \
>  	.type = IIO_VOLTAGE, \
>  	.indexed = 1, \
>  	.address = (_addr), \
> @@ -243,21 +244,24 @@ enum ams_ps_pl_seq {
>  	.event_spec = (_alarm) ? ams_voltage_events : NULL, \
>  	.scan_index = _scan_index, \
>  	.num_event_specs = (_alarm) ? ARRAY_SIZE(ams_voltage_events) : 0, \
> +	.datasheet_name = _name, \
>  }
>  
> -#define AMS_PS_CHAN_TEMP(_scan_index, _addr) \
> -	AMS_CHAN_TEMP(PS_SEQ(_scan_index), _addr)
> -#define AMS_PS_CHAN_VOLTAGE(_scan_index, _addr) \
> -	AMS_CHAN_VOLTAGE(PS_SEQ(_scan_index), _addr, true)
> +#define AMS_PS_CHAN_TEMP(_scan_index, _addr, _name) \
> +	AMS_CHAN_TEMP(PS_SEQ(_scan_index), _addr, _name)
> +#define AMS_PS_CHAN_VOLTAGE(_scan_index, _addr, _name) \
> +	AMS_CHAN_VOLTAGE(PS_SEQ(_scan_index), _addr, true, _name)
>  
> -#define AMS_PL_CHAN_TEMP(_scan_index, _addr) \
> -	AMS_CHAN_TEMP(PL_SEQ(_scan_index), _addr)
> -#define AMS_PL_CHAN_VOLTAGE(_scan_index, _addr, _alarm) \
> -	AMS_CHAN_VOLTAGE(PL_SEQ(_scan_index), _addr, _alarm)
> +#define AMS_PL_CHAN_TEMP(_scan_index, _addr, _name) \
> +	AMS_CHAN_TEMP(PL_SEQ(_scan_index), _addr, _name)
> +#define AMS_PL_CHAN_VOLTAGE(_scan_index, _addr, _alarm, _name) \
> +	AMS_CHAN_VOLTAGE(PL_SEQ(_scan_index), _addr, _alarm, _name)
>  #define AMS_PL_AUX_CHAN_VOLTAGE(_auxno) \
> -	AMS_CHAN_VOLTAGE(PL_SEQ(AMS_SEQ(_auxno)), AMS_REG_VAUX(_auxno), false)
> -#define AMS_CTRL_CHAN_VOLTAGE(_scan_index, _addr) \
> -	AMS_CHAN_VOLTAGE(PL_SEQ(AMS_SEQ(AMS_SEQ(_scan_index))), _addr, false)
> +	AMS_CHAN_VOLTAGE(PL_SEQ(AMS_SEQ(_auxno)), AMS_REG_VAUX(_auxno), false, \
> +			 "VAUX" #_auxno)
> +#define AMS_CTRL_CHAN_VOLTAGE(_scan_index, _addr, _name) \
> +	AMS_CHAN_VOLTAGE(PL_SEQ(AMS_SEQ(AMS_SEQ(_scan_index))), _addr, false, \
> +			 _name)
>  
>  /**
>   * struct ams - This structure contains necessary state for xilinx-ams to operate
> @@ -505,6 +509,12 @@ static int ams_init_device(struct ams *ams)
>  	return 0;
>  }
>  
> +static int ams_read_label(struct iio_dev *indio_dev,
> +			  struct iio_chan_spec const *chan, char *label)
> +{
> +	return sysfs_emit(label, "%s\n", chan->datasheet_name);
> +}
> +
>  static int ams_enable_single_channel(struct ams *ams, unsigned int offset)
>  {
>  	u8 channel_num;
> @@ -1116,37 +1126,37 @@ static const struct iio_event_spec ams_voltage_events[] = {
>  };
>  
>  static const struct iio_chan_spec ams_ps_channels[] = {
> -	AMS_PS_CHAN_TEMP(AMS_SEQ_TEMP, AMS_TEMP),
> -	AMS_PS_CHAN_TEMP(AMS_SEQ_TEMP_REMOTE, AMS_TEMP_REMOTE),
> -	AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY1, AMS_SUPPLY1),
> -	AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY2, AMS_SUPPLY2),
> -	AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY3, AMS_SUPPLY3),
> -	AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY4, AMS_SUPPLY4),
> -	AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY5, AMS_SUPPLY5),
> -	AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY6, AMS_SUPPLY6),
> -	AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY7, AMS_SUPPLY7),
> -	AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY8, AMS_SUPPLY8),
> -	AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY9, AMS_SUPPLY9),
> -	AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY10, AMS_SUPPLY10),
> -	AMS_PS_CHAN_VOLTAGE(AMS_SEQ_VCCAMS, AMS_VCCAMS),
> +	AMS_PS_CHAN_TEMP(AMS_SEQ_TEMP, AMS_TEMP, "Temp_LPD"),
> +	AMS_PS_CHAN_TEMP(AMS_SEQ_TEMP_REMOTE, AMS_TEMP_REMOTE, "Temp_FPD"),
> +	AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY1, AMS_SUPPLY1, "VCC_PSINTLP"),
> +	AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY2, AMS_SUPPLY2, "VCC_PSINTFP"),
> +	AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY3, AMS_SUPPLY3, "VCC_PSAUX"),
> +	AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY4, AMS_SUPPLY4, "VCC_PSDDR"),
> +	AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY5, AMS_SUPPLY5, "VCC_PSIO3"),
> +	AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY6, AMS_SUPPLY6, "VCC_PSIO0"),
> +	AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY7, AMS_SUPPLY7, "VCC_PSIO1"),
> +	AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY8, AMS_SUPPLY8, "VCC_PSIO2"),
> +	AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY9, AMS_SUPPLY9, "PS_MGTRAVCC"),
> +	AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY10, AMS_SUPPLY10, "PS_MGTRAVTT"),
> +	AMS_PS_CHAN_VOLTAGE(AMS_SEQ_VCCAMS, AMS_VCCAMS, "VCC_PSADC"),
>  };
>  
>  static const struct iio_chan_spec ams_pl_channels[] = {
> -	AMS_PL_CHAN_TEMP(AMS_SEQ_TEMP, AMS_TEMP),
> -	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY1, AMS_SUPPLY1, true),
> -	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY2, AMS_SUPPLY2, true),
> -	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_VREFP, AMS_VREFP, false),
> -	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_VREFN, AMS_VREFN, false),
> -	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY3, AMS_SUPPLY3, true),
> -	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY4, AMS_SUPPLY4, true),
> -	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY5, AMS_SUPPLY5, true),
> -	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY6, AMS_SUPPLY6, true),
> -	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_VCCAMS, AMS_VCCAMS, true),
> -	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_VP_VN, AMS_VP_VN, false),
> -	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY7, AMS_SUPPLY7, true),
> -	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY8, AMS_SUPPLY8, true),
> -	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY9, AMS_SUPPLY9, true),
> -	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY10, AMS_SUPPLY10, true),
> +	AMS_PL_CHAN_TEMP(AMS_SEQ_TEMP, AMS_TEMP, "Temp_PL"),
> +	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY1, AMS_SUPPLY1, true, "VCCINT"),
> +	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY2, AMS_SUPPLY2, true, "VCCAUX"),
> +	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_VREFP, AMS_VREFP, false, "VREFP"),
> +	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_VREFN, AMS_VREFN, false, "VREFN"),
> +	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY3, AMS_SUPPLY3, true, "VCCBRAM"),
> +	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY4, AMS_SUPPLY4, true, "VCC_PSINTLP"),
> +	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY5, AMS_SUPPLY5, true, "VCC_PSINTFP"),
> +	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY6, AMS_SUPPLY6, true, "VCC_PSAUX"),
> +	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_VCCAMS, AMS_VCCAMS, true, "VCCAMS"),
> +	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_VP_VN, AMS_VP_VN, false, "VP_VN"),
> +	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY7, AMS_SUPPLY7, true, "VUser0"),
> +	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY8, AMS_SUPPLY8, true, "VUser1"),
> +	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY9, AMS_SUPPLY9, true, "VUser2"),
> +	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY10, AMS_SUPPLY10, true, "VUser3"),
>  	AMS_PL_AUX_CHAN_VOLTAGE(0),
>  	AMS_PL_AUX_CHAN_VOLTAGE(1),
>  	AMS_PL_AUX_CHAN_VOLTAGE(2),
> @@ -1166,13 +1176,13 @@ static const struct iio_chan_spec ams_pl_channels[] = {
>  };
>  
>  static const struct iio_chan_spec ams_ctrl_channels[] = {
> -	AMS_CTRL_CHAN_VOLTAGE(AMS_SEQ_VCC_PSPLL, AMS_VCC_PSPLL0),
> -	AMS_CTRL_CHAN_VOLTAGE(AMS_SEQ_VCC_PSBATT, AMS_VCC_PSPLL3),
> -	AMS_CTRL_CHAN_VOLTAGE(AMS_SEQ_VCCINT, AMS_VCCINT),
> -	AMS_CTRL_CHAN_VOLTAGE(AMS_SEQ_VCCBRAM, AMS_VCCBRAM),
> -	AMS_CTRL_CHAN_VOLTAGE(AMS_SEQ_VCCAUX, AMS_VCCAUX),
> -	AMS_CTRL_CHAN_VOLTAGE(AMS_SEQ_PSDDRPLL, AMS_PSDDRPLL),
> -	AMS_CTRL_CHAN_VOLTAGE(AMS_SEQ_INTDDR, AMS_PSINTFPDDR),
> +	AMS_CTRL_CHAN_VOLTAGE(AMS_SEQ_VCC_PSPLL, AMS_VCC_PSPLL0, "VCC_PSPLL"),
> +	AMS_CTRL_CHAN_VOLTAGE(AMS_SEQ_VCC_PSBATT, AMS_VCC_PSPLL3, "VCC_PSBATT"),
> +	AMS_CTRL_CHAN_VOLTAGE(AMS_SEQ_VCCINT, AMS_VCCINT, "VCCINT"),
> +	AMS_CTRL_CHAN_VOLTAGE(AMS_SEQ_VCCBRAM, AMS_VCCBRAM, "VCCBRAM"),
> +	AMS_CTRL_CHAN_VOLTAGE(AMS_SEQ_VCCAUX, AMS_VCCAUX, "VCCAUX"),
> +	AMS_CTRL_CHAN_VOLTAGE(AMS_SEQ_PSDDRPLL, AMS_PSDDRPLL, "VCC_PSDDR_PLL"),
> +	AMS_CTRL_CHAN_VOLTAGE(AMS_SEQ_INTDDR, AMS_PSINTFPDDR, "VCC_PSINTFP_DDR"),
>  };
>  
>  static int ams_get_ext_chan(struct fwnode_handle *chan_node,
> @@ -1336,6 +1346,7 @@ static int ams_parse_firmware(struct iio_dev *indio_dev)
>  }
>  
>  static const struct iio_info iio_ams_info = {
> +	.read_label = ams_read_label,
>  	.read_raw = &ams_read_raw,
>  	.read_event_config = &ams_read_event_config,
>  	.write_event_config = &ams_write_event_config,
Jonathan Cameron June 30, 2024, 10:41 a.m. UTC | #2 
On Sun, 23 Jun 2024 12:59:01 +0100
Jonathan Cameron <jic23@kernel.org> wrote:

> On Thu, 20 Jun 2024 16:48:42 -0400
> Sean Anderson <sean.anderson@linux.dev> wrote:
> 
> > Label all the channels using names from the reference manual. Some of
> > the "control" channels are duplicates of other channels. The reference
> > manual describes it like:
> >   
> > > The AMS register set includes several measurement registers that are
> > > written to by the PS SYSMON unit using the single-channel mode
> > > (sequencer off). These voltage measurements are performed using the
> > > unipolar sampling circuit with a 0 to 3V range and do not have alarms
> > > or minimum/maximum registers.    
> > 
> > So I think these really are measuring the same voltages but in a
> > different location. In which case, sharing labels makes sense to me.
> > 
> > Signed-off-by: Sean Anderson <sean.anderson@linux.dev>  
> Hi Sean,
> 
> This LGTM, but I'd like some feedback from others more familiar with the
> driver and device before I apply it.  If everyone is too busy and
> I don't get any replies for a few weeks I may pick it up anyway.
> 
> Feel free to poke me if I seem to have forgotten in say 2 weeks.
Long enough. Applied.
> 
> 
> Jonathan
> 
> > ---
> > 
> >  drivers/iio/adc/xilinx-ams.c | 107 +++++++++++++++++++----------------
> >  1 file changed, 59 insertions(+), 48 deletions(-)
> > 
> > diff --git a/drivers/iio/adc/xilinx-ams.c b/drivers/iio/adc/xilinx-ams.c
> > index aa05f24931f9..f051358d6b50 100644
> > --- a/drivers/iio/adc/xilinx-ams.c
> > +++ b/drivers/iio/adc/xilinx-ams.c
> > @@ -222,7 +222,7 @@ enum ams_ps_pl_seq {
> >  #define PL_SEQ(x)		(AMS_PS_SEQ_MAX + (x))
> >  #define AMS_CTRL_SEQ_BASE	(AMS_PS_SEQ_MAX * 3)
> >  
> > -#define AMS_CHAN_TEMP(_scan_index, _addr) { \
> > +#define AMS_CHAN_TEMP(_scan_index, _addr, _name) { \
> >  	.type = IIO_TEMP, \
> >  	.indexed = 1, \
> >  	.address = (_addr), \
> > @@ -232,9 +232,10 @@ enum ams_ps_pl_seq {
> >  	.event_spec = ams_temp_events, \
> >  	.scan_index = _scan_index, \
> >  	.num_event_specs = ARRAY_SIZE(ams_temp_events), \
> > +	.datasheet_name = _name, \
> >  }
> >  
> > -#define AMS_CHAN_VOLTAGE(_scan_index, _addr, _alarm) { \
> > +#define AMS_CHAN_VOLTAGE(_scan_index, _addr, _alarm, _name) { \
> >  	.type = IIO_VOLTAGE, \
> >  	.indexed = 1, \
> >  	.address = (_addr), \
> > @@ -243,21 +244,24 @@ enum ams_ps_pl_seq {
> >  	.event_spec = (_alarm) ? ams_voltage_events : NULL, \
> >  	.scan_index = _scan_index, \
> >  	.num_event_specs = (_alarm) ? ARRAY_SIZE(ams_voltage_events) : 0, \
> > +	.datasheet_name = _name, \
> >  }
> >  
> > -#define AMS_PS_CHAN_TEMP(_scan_index, _addr) \
> > -	AMS_CHAN_TEMP(PS_SEQ(_scan_index), _addr)
> > -#define AMS_PS_CHAN_VOLTAGE(_scan_index, _addr) \
> > -	AMS_CHAN_VOLTAGE(PS_SEQ(_scan_index), _addr, true)
> > +#define AMS_PS_CHAN_TEMP(_scan_index, _addr, _name) \
> > +	AMS_CHAN_TEMP(PS_SEQ(_scan_index), _addr, _name)
> > +#define AMS_PS_CHAN_VOLTAGE(_scan_index, _addr, _name) \
> > +	AMS_CHAN_VOLTAGE(PS_SEQ(_scan_index), _addr, true, _name)
> >  
> > -#define AMS_PL_CHAN_TEMP(_scan_index, _addr) \
> > -	AMS_CHAN_TEMP(PL_SEQ(_scan_index), _addr)
> > -#define AMS_PL_CHAN_VOLTAGE(_scan_index, _addr, _alarm) \
> > -	AMS_CHAN_VOLTAGE(PL_SEQ(_scan_index), _addr, _alarm)
> > +#define AMS_PL_CHAN_TEMP(_scan_index, _addr, _name) \
> > +	AMS_CHAN_TEMP(PL_SEQ(_scan_index), _addr, _name)
> > +#define AMS_PL_CHAN_VOLTAGE(_scan_index, _addr, _alarm, _name) \
> > +	AMS_CHAN_VOLTAGE(PL_SEQ(_scan_index), _addr, _alarm, _name)
> >  #define AMS_PL_AUX_CHAN_VOLTAGE(_auxno) \
> > -	AMS_CHAN_VOLTAGE(PL_SEQ(AMS_SEQ(_auxno)), AMS_REG_VAUX(_auxno), false)
> > -#define AMS_CTRL_CHAN_VOLTAGE(_scan_index, _addr) \
> > -	AMS_CHAN_VOLTAGE(PL_SEQ(AMS_SEQ(AMS_SEQ(_scan_index))), _addr, false)
> > +	AMS_CHAN_VOLTAGE(PL_SEQ(AMS_SEQ(_auxno)), AMS_REG_VAUX(_auxno), false, \
> > +			 "VAUX" #_auxno)
> > +#define AMS_CTRL_CHAN_VOLTAGE(_scan_index, _addr, _name) \
> > +	AMS_CHAN_VOLTAGE(PL_SEQ(AMS_SEQ(AMS_SEQ(_scan_index))), _addr, false, \
> > +			 _name)
> >  
> >  /**
> >   * struct ams - This structure contains necessary state for xilinx-ams to operate
> > @@ -505,6 +509,12 @@ static int ams_init_device(struct ams *ams)
> >  	return 0;
> >  }
> >  
> > +static int ams_read_label(struct iio_dev *indio_dev,
> > +			  struct iio_chan_spec const *chan, char *label)
> > +{
> > +	return sysfs_emit(label, "%s\n", chan->datasheet_name);
> > +}
> > +
> >  static int ams_enable_single_channel(struct ams *ams, unsigned int offset)
> >  {
> >  	u8 channel_num;
> > @@ -1116,37 +1126,37 @@ static const struct iio_event_spec ams_voltage_events[] = {
> >  };
> >  
> >  static const struct iio_chan_spec ams_ps_channels[] = {
> > -	AMS_PS_CHAN_TEMP(AMS_SEQ_TEMP, AMS_TEMP),
> > -	AMS_PS_CHAN_TEMP(AMS_SEQ_TEMP_REMOTE, AMS_TEMP_REMOTE),
> > -	AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY1, AMS_SUPPLY1),
> > -	AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY2, AMS_SUPPLY2),
> > -	AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY3, AMS_SUPPLY3),
> > -	AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY4, AMS_SUPPLY4),
> > -	AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY5, AMS_SUPPLY5),
> > -	AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY6, AMS_SUPPLY6),
> > -	AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY7, AMS_SUPPLY7),
> > -	AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY8, AMS_SUPPLY8),
> > -	AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY9, AMS_SUPPLY9),
> > -	AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY10, AMS_SUPPLY10),
> > -	AMS_PS_CHAN_VOLTAGE(AMS_SEQ_VCCAMS, AMS_VCCAMS),
> > +	AMS_PS_CHAN_TEMP(AMS_SEQ_TEMP, AMS_TEMP, "Temp_LPD"),
> > +	AMS_PS_CHAN_TEMP(AMS_SEQ_TEMP_REMOTE, AMS_TEMP_REMOTE, "Temp_FPD"),
> > +	AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY1, AMS_SUPPLY1, "VCC_PSINTLP"),
> > +	AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY2, AMS_SUPPLY2, "VCC_PSINTFP"),
> > +	AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY3, AMS_SUPPLY3, "VCC_PSAUX"),
> > +	AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY4, AMS_SUPPLY4, "VCC_PSDDR"),
> > +	AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY5, AMS_SUPPLY5, "VCC_PSIO3"),
> > +	AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY6, AMS_SUPPLY6, "VCC_PSIO0"),
> > +	AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY7, AMS_SUPPLY7, "VCC_PSIO1"),
> > +	AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY8, AMS_SUPPLY8, "VCC_PSIO2"),
> > +	AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY9, AMS_SUPPLY9, "PS_MGTRAVCC"),
> > +	AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY10, AMS_SUPPLY10, "PS_MGTRAVTT"),
> > +	AMS_PS_CHAN_VOLTAGE(AMS_SEQ_VCCAMS, AMS_VCCAMS, "VCC_PSADC"),
> >  };
> >  
> >  static const struct iio_chan_spec ams_pl_channels[] = {
> > -	AMS_PL_CHAN_TEMP(AMS_SEQ_TEMP, AMS_TEMP),
> > -	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY1, AMS_SUPPLY1, true),
> > -	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY2, AMS_SUPPLY2, true),
> > -	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_VREFP, AMS_VREFP, false),
> > -	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_VREFN, AMS_VREFN, false),
> > -	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY3, AMS_SUPPLY3, true),
> > -	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY4, AMS_SUPPLY4, true),
> > -	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY5, AMS_SUPPLY5, true),
> > -	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY6, AMS_SUPPLY6, true),
> > -	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_VCCAMS, AMS_VCCAMS, true),
> > -	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_VP_VN, AMS_VP_VN, false),
> > -	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY7, AMS_SUPPLY7, true),
> > -	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY8, AMS_SUPPLY8, true),
> > -	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY9, AMS_SUPPLY9, true),
> > -	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY10, AMS_SUPPLY10, true),
> > +	AMS_PL_CHAN_TEMP(AMS_SEQ_TEMP, AMS_TEMP, "Temp_PL"),
> > +	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY1, AMS_SUPPLY1, true, "VCCINT"),
> > +	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY2, AMS_SUPPLY2, true, "VCCAUX"),
> > +	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_VREFP, AMS_VREFP, false, "VREFP"),
> > +	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_VREFN, AMS_VREFN, false, "VREFN"),
> > +	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY3, AMS_SUPPLY3, true, "VCCBRAM"),
> > +	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY4, AMS_SUPPLY4, true, "VCC_PSINTLP"),
> > +	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY5, AMS_SUPPLY5, true, "VCC_PSINTFP"),
> > +	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY6, AMS_SUPPLY6, true, "VCC_PSAUX"),
> > +	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_VCCAMS, AMS_VCCAMS, true, "VCCAMS"),
> > +	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_VP_VN, AMS_VP_VN, false, "VP_VN"),
> > +	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY7, AMS_SUPPLY7, true, "VUser0"),
> > +	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY8, AMS_SUPPLY8, true, "VUser1"),
> > +	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY9, AMS_SUPPLY9, true, "VUser2"),
> > +	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY10, AMS_SUPPLY10, true, "VUser3"),
> >  	AMS_PL_AUX_CHAN_VOLTAGE(0),
> >  	AMS_PL_AUX_CHAN_VOLTAGE(1),
> >  	AMS_PL_AUX_CHAN_VOLTAGE(2),
> > @@ -1166,13 +1176,13 @@ static const struct iio_chan_spec ams_pl_channels[] = {
> >  };
> >  
> >  static const struct iio_chan_spec ams_ctrl_channels[] = {
> > -	AMS_CTRL_CHAN_VOLTAGE(AMS_SEQ_VCC_PSPLL, AMS_VCC_PSPLL0),
> > -	AMS_CTRL_CHAN_VOLTAGE(AMS_SEQ_VCC_PSBATT, AMS_VCC_PSPLL3),
> > -	AMS_CTRL_CHAN_VOLTAGE(AMS_SEQ_VCCINT, AMS_VCCINT),
> > -	AMS_CTRL_CHAN_VOLTAGE(AMS_SEQ_VCCBRAM, AMS_VCCBRAM),
> > -	AMS_CTRL_CHAN_VOLTAGE(AMS_SEQ_VCCAUX, AMS_VCCAUX),
> > -	AMS_CTRL_CHAN_VOLTAGE(AMS_SEQ_PSDDRPLL, AMS_PSDDRPLL),
> > -	AMS_CTRL_CHAN_VOLTAGE(AMS_SEQ_INTDDR, AMS_PSINTFPDDR),
> > +	AMS_CTRL_CHAN_VOLTAGE(AMS_SEQ_VCC_PSPLL, AMS_VCC_PSPLL0, "VCC_PSPLL"),
> > +	AMS_CTRL_CHAN_VOLTAGE(AMS_SEQ_VCC_PSBATT, AMS_VCC_PSPLL3, "VCC_PSBATT"),
> > +	AMS_CTRL_CHAN_VOLTAGE(AMS_SEQ_VCCINT, AMS_VCCINT, "VCCINT"),
> > +	AMS_CTRL_CHAN_VOLTAGE(AMS_SEQ_VCCBRAM, AMS_VCCBRAM, "VCCBRAM"),
> > +	AMS_CTRL_CHAN_VOLTAGE(AMS_SEQ_VCCAUX, AMS_VCCAUX, "VCCAUX"),
> > +	AMS_CTRL_CHAN_VOLTAGE(AMS_SEQ_PSDDRPLL, AMS_PSDDRPLL, "VCC_PSDDR_PLL"),
> > +	AMS_CTRL_CHAN_VOLTAGE(AMS_SEQ_INTDDR, AMS_PSINTFPDDR, "VCC_PSINTFP_DDR"),
> >  };
> >  
> >  static int ams_get_ext_chan(struct fwnode_handle *chan_node,
> > @@ -1336,6 +1346,7 @@ static int ams_parse_firmware(struct iio_dev *indio_dev)
> >  }
> >  
> >  static const struct iio_info iio_ams_info = {
> > +	.read_label = ams_read_label,
> >  	.read_raw = &ams_read_raw,
> >  	.read_event_config = &ams_read_event_config,
> >  	.write_event_config = &ams_write_event_config,  
> 
>
O'Griofa, Conall July 17, 2024, 8:50 a.m. UTC | #3 
[AMD Official Use Only - AMD Internal Distribution Only]

Acked-by: O'Griofa, Conall <conall.ogriofa@amd.com>
Tested-by: O'Griofa, Conall <conall.ogriofa@amd.com>

Nice patch, thank you for taking the time to submit.

> -----Original Message-----
> From: Sean Anderson <sean.anderson@linux.dev>
> Sent: Thursday, June 20, 2024 9:49 PM
> To: Anand Ashok Dumbre <anand.ashok.dumbre@xilinx.com>; linux-
> iio@vger.kernel.org
> Cc: Jonathan Cameron <jic23@kernel.org>; Lars-Peter Clausen
> <lars@metafoo.de>; Simek, Michal <michal.simek@amd.com>; linux-
> kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org; Sean
> Anderson <sean.anderson@linux.dev>
> Subject: [PATCH] iio: xilinx-ams: Add labels
>
> Label all the channels using names from the reference manual. Some of
> the "control" channels are duplicates of other channels. The reference
> manual describes it like:
>
> > The AMS register set includes several measurement registers that are
> > written to by the PS SYSMON unit using the single-channel mode
> > (sequencer off). These voltage measurements are performed using the
> > unipolar sampling circuit with a 0 to 3V range and do not have alarms
> > or minimum/maximum registers.
>
> So I think these really are measuring the same voltages but in a
> different location. In which case, sharing labels makes sense to me.
>
> Signed-off-by: Sean Anderson <sean.anderson@linux.dev>
> ---
>
>  drivers/iio/adc/xilinx-ams.c | 107 +++++++++++++++++++----------------
>  1 file changed, 59 insertions(+), 48 deletions(-)
>
> diff --git a/drivers/iio/adc/xilinx-ams.c b/drivers/iio/adc/xilinx-ams.c
> index aa05f24931f9..f051358d6b50 100644
> --- a/drivers/iio/adc/xilinx-ams.c
> +++ b/drivers/iio/adc/xilinx-ams.c
> @@ -222,7 +222,7 @@ enum ams_ps_pl_seq {
>  #define PL_SEQ(x)            (AMS_PS_SEQ_MAX + (x))
>  #define AMS_CTRL_SEQ_BASE    (AMS_PS_SEQ_MAX * 3)
>
> -#define AMS_CHAN_TEMP(_scan_index, _addr) { \
> +#define AMS_CHAN_TEMP(_scan_index, _addr, _name) { \
>       .type = IIO_TEMP, \
>       .indexed = 1, \
>       .address = (_addr), \
> @@ -232,9 +232,10 @@ enum ams_ps_pl_seq {
>       .event_spec = ams_temp_events, \
>       .scan_index = _scan_index, \
>       .num_event_specs = ARRAY_SIZE(ams_temp_events), \
> +     .datasheet_name = _name, \
>  }
>
> -#define AMS_CHAN_VOLTAGE(_scan_index, _addr, _alarm) { \
> +#define AMS_CHAN_VOLTAGE(_scan_index, _addr, _alarm, _name) { \
>       .type = IIO_VOLTAGE, \
>       .indexed = 1, \
>       .address = (_addr), \
> @@ -243,21 +244,24 @@ enum ams_ps_pl_seq {
>       .event_spec = (_alarm) ? ams_voltage_events : NULL, \
>       .scan_index = _scan_index, \
>       .num_event_specs = (_alarm) ? ARRAY_SIZE(ams_voltage_events) : 0,
> \
> +     .datasheet_name = _name, \
>  }
>
> -#define AMS_PS_CHAN_TEMP(_scan_index, _addr) \
> -     AMS_CHAN_TEMP(PS_SEQ(_scan_index), _addr)
> -#define AMS_PS_CHAN_VOLTAGE(_scan_index, _addr) \
> -     AMS_CHAN_VOLTAGE(PS_SEQ(_scan_index), _addr, true)
> +#define AMS_PS_CHAN_TEMP(_scan_index, _addr, _name) \
> +     AMS_CHAN_TEMP(PS_SEQ(_scan_index), _addr, _name)
> +#define AMS_PS_CHAN_VOLTAGE(_scan_index, _addr, _name) \
> +     AMS_CHAN_VOLTAGE(PS_SEQ(_scan_index), _addr, true, _name)
>
> -#define AMS_PL_CHAN_TEMP(_scan_index, _addr) \
> -     AMS_CHAN_TEMP(PL_SEQ(_scan_index), _addr)
> -#define AMS_PL_CHAN_VOLTAGE(_scan_index, _addr, _alarm) \
> -     AMS_CHAN_VOLTAGE(PL_SEQ(_scan_index), _addr, _alarm)
> +#define AMS_PL_CHAN_TEMP(_scan_index, _addr, _name) \
> +     AMS_CHAN_TEMP(PL_SEQ(_scan_index), _addr, _name)
> +#define AMS_PL_CHAN_VOLTAGE(_scan_index, _addr, _alarm, _name) \
> +     AMS_CHAN_VOLTAGE(PL_SEQ(_scan_index), _addr, _alarm, _name)
>  #define AMS_PL_AUX_CHAN_VOLTAGE(_auxno) \
> -     AMS_CHAN_VOLTAGE(PL_SEQ(AMS_SEQ(_auxno)),
> AMS_REG_VAUX(_auxno), false)
> -#define AMS_CTRL_CHAN_VOLTAGE(_scan_index, _addr) \
> -     AMS_CHAN_VOLTAGE(PL_SEQ(AMS_SEQ(AMS_SEQ(_scan_index))),
> _addr, false)
> +     AMS_CHAN_VOLTAGE(PL_SEQ(AMS_SEQ(_auxno)),
> AMS_REG_VAUX(_auxno), false, \
> +                      "VAUX" #_auxno)
> +#define AMS_CTRL_CHAN_VOLTAGE(_scan_index, _addr, _name) \
> +     AMS_CHAN_VOLTAGE(PL_SEQ(AMS_SEQ(AMS_SEQ(_scan_index))),
> _addr, false, \
> +                      _name)
>
>  /**
>   * struct ams - This structure contains necessary state for xilinx-ams to
> operate
> @@ -505,6 +509,12 @@ static int ams_init_device(struct ams *ams)
>       return 0;
>  }
>
> +static int ams_read_label(struct iio_dev *indio_dev,
> +                       struct iio_chan_spec const *chan, char *label)
> +{
> +     return sysfs_emit(label, "%s\n", chan->datasheet_name);
> +}
> +
>  static int ams_enable_single_channel(struct ams *ams, unsigned int offset)
>  {
>       u8 channel_num;
> @@ -1116,37 +1126,37 @@ static const struct iio_event_spec
> ams_voltage_events[] = {
>  };
>
>  static const struct iio_chan_spec ams_ps_channels[] = {
> -     AMS_PS_CHAN_TEMP(AMS_SEQ_TEMP, AMS_TEMP),
> -     AMS_PS_CHAN_TEMP(AMS_SEQ_TEMP_REMOTE,
> AMS_TEMP_REMOTE),
> -     AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY1, AMS_SUPPLY1),
> -     AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY2, AMS_SUPPLY2),
> -     AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY3, AMS_SUPPLY3),
> -     AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY4, AMS_SUPPLY4),
> -     AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY5, AMS_SUPPLY5),
> -     AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY6, AMS_SUPPLY6),
> -     AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY7, AMS_SUPPLY7),
> -     AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY8, AMS_SUPPLY8),
> -     AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY9, AMS_SUPPLY9),
> -     AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY10, AMS_SUPPLY10),
> -     AMS_PS_CHAN_VOLTAGE(AMS_SEQ_VCCAMS, AMS_VCCAMS),
> +     AMS_PS_CHAN_TEMP(AMS_SEQ_TEMP, AMS_TEMP, "Temp_LPD"),
> +     AMS_PS_CHAN_TEMP(AMS_SEQ_TEMP_REMOTE,
> AMS_TEMP_REMOTE, "Temp_FPD"),
> +     AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY1, AMS_SUPPLY1,
> "VCC_PSINTLP"),
> +     AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY2, AMS_SUPPLY2,
> "VCC_PSINTFP"),
> +     AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY3, AMS_SUPPLY3,
> "VCC_PSAUX"),
> +     AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY4, AMS_SUPPLY4,
> "VCC_PSDDR"),
> +     AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY5, AMS_SUPPLY5,
> "VCC_PSIO3"),
> +     AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY6, AMS_SUPPLY6,
> "VCC_PSIO0"),
> +     AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY7, AMS_SUPPLY7,
> "VCC_PSIO1"),
> +     AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY8, AMS_SUPPLY8,
> "VCC_PSIO2"),
> +     AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY9, AMS_SUPPLY9,
> "PS_MGTRAVCC"),
> +     AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY10, AMS_SUPPLY10,
> "PS_MGTRAVTT"),
> +     AMS_PS_CHAN_VOLTAGE(AMS_SEQ_VCCAMS, AMS_VCCAMS,
> "VCC_PSADC"),
>  };
>
>  static const struct iio_chan_spec ams_pl_channels[] = {
> -     AMS_PL_CHAN_TEMP(AMS_SEQ_TEMP, AMS_TEMP),
> -     AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY1, AMS_SUPPLY1, true),
> -     AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY2, AMS_SUPPLY2, true),
> -     AMS_PL_CHAN_VOLTAGE(AMS_SEQ_VREFP, AMS_VREFP, false),
> -     AMS_PL_CHAN_VOLTAGE(AMS_SEQ_VREFN, AMS_VREFN, false),
> -     AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY3, AMS_SUPPLY3, true),
> -     AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY4, AMS_SUPPLY4, true),
> -     AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY5, AMS_SUPPLY5, true),
> -     AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY6, AMS_SUPPLY6, true),
> -     AMS_PL_CHAN_VOLTAGE(AMS_SEQ_VCCAMS, AMS_VCCAMS, true),
> -     AMS_PL_CHAN_VOLTAGE(AMS_SEQ_VP_VN, AMS_VP_VN, false),
> -     AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY7, AMS_SUPPLY7, true),
> -     AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY8, AMS_SUPPLY8, true),
> -     AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY9, AMS_SUPPLY9, true),
> -     AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY10, AMS_SUPPLY10,
> true),
> +     AMS_PL_CHAN_TEMP(AMS_SEQ_TEMP, AMS_TEMP, "Temp_PL"),
> +     AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY1, AMS_SUPPLY1, true,
> "VCCINT"),
> +     AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY2, AMS_SUPPLY2, true,
> "VCCAUX"),
> +     AMS_PL_CHAN_VOLTAGE(AMS_SEQ_VREFP, AMS_VREFP, false,
> "VREFP"),
> +     AMS_PL_CHAN_VOLTAGE(AMS_SEQ_VREFN, AMS_VREFN, false,
> "VREFN"),
> +     AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY3, AMS_SUPPLY3, true,
> "VCCBRAM"),
> +     AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY4, AMS_SUPPLY4, true,
> "VCC_PSINTLP"),
> +     AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY5, AMS_SUPPLY5, true,
> "VCC_PSINTFP"),
> +     AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY6, AMS_SUPPLY6, true,
> "VCC_PSAUX"),
> +     AMS_PL_CHAN_VOLTAGE(AMS_SEQ_VCCAMS, AMS_VCCAMS, true,
> "VCCAMS"),
> +     AMS_PL_CHAN_VOLTAGE(AMS_SEQ_VP_VN, AMS_VP_VN, false,
> "VP_VN"),
> +     AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY7, AMS_SUPPLY7, true,
> "VUser0"),
> +     AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY8, AMS_SUPPLY8, true,
> "VUser1"),
> +     AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY9, AMS_SUPPLY9, true,
> "VUser2"),
> +     AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY10, AMS_SUPPLY10, true,
> "VUser3"),
>       AMS_PL_AUX_CHAN_VOLTAGE(0),
>       AMS_PL_AUX_CHAN_VOLTAGE(1),
>       AMS_PL_AUX_CHAN_VOLTAGE(2),
> @@ -1166,13 +1176,13 @@ static const struct iio_chan_spec
> ams_pl_channels[] = {
>  };
>
>  static const struct iio_chan_spec ams_ctrl_channels[] = {
> -     AMS_CTRL_CHAN_VOLTAGE(AMS_SEQ_VCC_PSPLL,
> AMS_VCC_PSPLL0),
> -     AMS_CTRL_CHAN_VOLTAGE(AMS_SEQ_VCC_PSBATT,
> AMS_VCC_PSPLL3),
> -     AMS_CTRL_CHAN_VOLTAGE(AMS_SEQ_VCCINT, AMS_VCCINT),
> -     AMS_CTRL_CHAN_VOLTAGE(AMS_SEQ_VCCBRAM, AMS_VCCBRAM),
> -     AMS_CTRL_CHAN_VOLTAGE(AMS_SEQ_VCCAUX, AMS_VCCAUX),
> -     AMS_CTRL_CHAN_VOLTAGE(AMS_SEQ_PSDDRPLL, AMS_PSDDRPLL),
> -     AMS_CTRL_CHAN_VOLTAGE(AMS_SEQ_INTDDR, AMS_PSINTFPDDR),
> +     AMS_CTRL_CHAN_VOLTAGE(AMS_SEQ_VCC_PSPLL, AMS_VCC_PSPLL0,
> "VCC_PSPLL"),
> +     AMS_CTRL_CHAN_VOLTAGE(AMS_SEQ_VCC_PSBATT,
> AMS_VCC_PSPLL3, "VCC_PSBATT"),
> +     AMS_CTRL_CHAN_VOLTAGE(AMS_SEQ_VCCINT, AMS_VCCINT,
> "VCCINT"),
> +     AMS_CTRL_CHAN_VOLTAGE(AMS_SEQ_VCCBRAM, AMS_VCCBRAM,
> "VCCBRAM"),
> +     AMS_CTRL_CHAN_VOLTAGE(AMS_SEQ_VCCAUX, AMS_VCCAUX,
> "VCCAUX"),
> +     AMS_CTRL_CHAN_VOLTAGE(AMS_SEQ_PSDDRPLL, AMS_PSDDRPLL,
> "VCC_PSDDR_PLL"),
> +     AMS_CTRL_CHAN_VOLTAGE(AMS_SEQ_INTDDR, AMS_PSINTFPDDR,
> "VCC_PSINTFP_DDR"),
>  };
>
>  static int ams_get_ext_chan(struct fwnode_handle *chan_node,
> @@ -1336,6 +1346,7 @@ static int ams_parse_firmware(struct iio_dev
> *indio_dev)
>  }
>
>  static const struct iio_info iio_ams_info = {
> +     .read_label = ams_read_label,
>       .read_raw = &ams_read_raw,
>       .read_event_config = &ams_read_event_config,
>       .write_event_config = &ams_write_event_config,
> --
> 2.35.1.1320.gc452695387.dirty
diff mbox series

Patch

diff --git a/drivers/iio/adc/xilinx-ams.c b/drivers/iio/adc/xilinx-ams.c
index aa05f24931f9..f051358d6b50 100644
--- a/drivers/iio/adc/xilinx-ams.c
+++ b/drivers/iio/adc/xilinx-ams.c
@@ -222,7 +222,7 @@  enum ams_ps_pl_seq {
 #define PL_SEQ(x)		(AMS_PS_SEQ_MAX + (x))
 #define AMS_CTRL_SEQ_BASE	(AMS_PS_SEQ_MAX * 3)
 
-#define AMS_CHAN_TEMP(_scan_index, _addr) { \
+#define AMS_CHAN_TEMP(_scan_index, _addr, _name) { \
 	.type = IIO_TEMP, \
 	.indexed = 1, \
 	.address = (_addr), \
@@ -232,9 +232,10 @@  enum ams_ps_pl_seq {
 	.event_spec = ams_temp_events, \
 	.scan_index = _scan_index, \
 	.num_event_specs = ARRAY_SIZE(ams_temp_events), \
+	.datasheet_name = _name, \
 }
 
-#define AMS_CHAN_VOLTAGE(_scan_index, _addr, _alarm) { \
+#define AMS_CHAN_VOLTAGE(_scan_index, _addr, _alarm, _name) { \
 	.type = IIO_VOLTAGE, \
 	.indexed = 1, \
 	.address = (_addr), \
@@ -243,21 +244,24 @@  enum ams_ps_pl_seq {
 	.event_spec = (_alarm) ? ams_voltage_events : NULL, \
 	.scan_index = _scan_index, \
 	.num_event_specs = (_alarm) ? ARRAY_SIZE(ams_voltage_events) : 0, \
+	.datasheet_name = _name, \
 }
 
-#define AMS_PS_CHAN_TEMP(_scan_index, _addr) \
-	AMS_CHAN_TEMP(PS_SEQ(_scan_index), _addr)
-#define AMS_PS_CHAN_VOLTAGE(_scan_index, _addr) \
-	AMS_CHAN_VOLTAGE(PS_SEQ(_scan_index), _addr, true)
+#define AMS_PS_CHAN_TEMP(_scan_index, _addr, _name) \
+	AMS_CHAN_TEMP(PS_SEQ(_scan_index), _addr, _name)
+#define AMS_PS_CHAN_VOLTAGE(_scan_index, _addr, _name) \
+	AMS_CHAN_VOLTAGE(PS_SEQ(_scan_index), _addr, true, _name)
 
-#define AMS_PL_CHAN_TEMP(_scan_index, _addr) \
-	AMS_CHAN_TEMP(PL_SEQ(_scan_index), _addr)
-#define AMS_PL_CHAN_VOLTAGE(_scan_index, _addr, _alarm) \
-	AMS_CHAN_VOLTAGE(PL_SEQ(_scan_index), _addr, _alarm)
+#define AMS_PL_CHAN_TEMP(_scan_index, _addr, _name) \
+	AMS_CHAN_TEMP(PL_SEQ(_scan_index), _addr, _name)
+#define AMS_PL_CHAN_VOLTAGE(_scan_index, _addr, _alarm, _name) \
+	AMS_CHAN_VOLTAGE(PL_SEQ(_scan_index), _addr, _alarm, _name)
 #define AMS_PL_AUX_CHAN_VOLTAGE(_auxno) \
-	AMS_CHAN_VOLTAGE(PL_SEQ(AMS_SEQ(_auxno)), AMS_REG_VAUX(_auxno), false)
-#define AMS_CTRL_CHAN_VOLTAGE(_scan_index, _addr) \
-	AMS_CHAN_VOLTAGE(PL_SEQ(AMS_SEQ(AMS_SEQ(_scan_index))), _addr, false)
+	AMS_CHAN_VOLTAGE(PL_SEQ(AMS_SEQ(_auxno)), AMS_REG_VAUX(_auxno), false, \
+			 "VAUX" #_auxno)
+#define AMS_CTRL_CHAN_VOLTAGE(_scan_index, _addr, _name) \
+	AMS_CHAN_VOLTAGE(PL_SEQ(AMS_SEQ(AMS_SEQ(_scan_index))), _addr, false, \
+			 _name)
 
 /**
  * struct ams - This structure contains necessary state for xilinx-ams to operate
@@ -505,6 +509,12 @@  static int ams_init_device(struct ams *ams)
 	return 0;
 }
 
+static int ams_read_label(struct iio_dev *indio_dev,
+			  struct iio_chan_spec const *chan, char *label)
+{
+	return sysfs_emit(label, "%s\n", chan->datasheet_name);
+}
+
 static int ams_enable_single_channel(struct ams *ams, unsigned int offset)
 {
 	u8 channel_num;
@@ -1116,37 +1126,37 @@  static const struct iio_event_spec ams_voltage_events[] = {
 };
 
 static const struct iio_chan_spec ams_ps_channels[] = {
-	AMS_PS_CHAN_TEMP(AMS_SEQ_TEMP, AMS_TEMP),
-	AMS_PS_CHAN_TEMP(AMS_SEQ_TEMP_REMOTE, AMS_TEMP_REMOTE),
-	AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY1, AMS_SUPPLY1),
-	AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY2, AMS_SUPPLY2),
-	AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY3, AMS_SUPPLY3),
-	AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY4, AMS_SUPPLY4),
-	AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY5, AMS_SUPPLY5),
-	AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY6, AMS_SUPPLY6),
-	AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY7, AMS_SUPPLY7),
-	AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY8, AMS_SUPPLY8),
-	AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY9, AMS_SUPPLY9),
-	AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY10, AMS_SUPPLY10),
-	AMS_PS_CHAN_VOLTAGE(AMS_SEQ_VCCAMS, AMS_VCCAMS),
+	AMS_PS_CHAN_TEMP(AMS_SEQ_TEMP, AMS_TEMP, "Temp_LPD"),
+	AMS_PS_CHAN_TEMP(AMS_SEQ_TEMP_REMOTE, AMS_TEMP_REMOTE, "Temp_FPD"),
+	AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY1, AMS_SUPPLY1, "VCC_PSINTLP"),
+	AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY2, AMS_SUPPLY2, "VCC_PSINTFP"),
+	AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY3, AMS_SUPPLY3, "VCC_PSAUX"),
+	AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY4, AMS_SUPPLY4, "VCC_PSDDR"),
+	AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY5, AMS_SUPPLY5, "VCC_PSIO3"),
+	AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY6, AMS_SUPPLY6, "VCC_PSIO0"),
+	AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY7, AMS_SUPPLY7, "VCC_PSIO1"),
+	AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY8, AMS_SUPPLY8, "VCC_PSIO2"),
+	AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY9, AMS_SUPPLY9, "PS_MGTRAVCC"),
+	AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY10, AMS_SUPPLY10, "PS_MGTRAVTT"),
+	AMS_PS_CHAN_VOLTAGE(AMS_SEQ_VCCAMS, AMS_VCCAMS, "VCC_PSADC"),
 };
 
 static const struct iio_chan_spec ams_pl_channels[] = {
-	AMS_PL_CHAN_TEMP(AMS_SEQ_TEMP, AMS_TEMP),
-	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY1, AMS_SUPPLY1, true),
-	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY2, AMS_SUPPLY2, true),
-	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_VREFP, AMS_VREFP, false),
-	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_VREFN, AMS_VREFN, false),
-	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY3, AMS_SUPPLY3, true),
-	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY4, AMS_SUPPLY4, true),
-	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY5, AMS_SUPPLY5, true),
-	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY6, AMS_SUPPLY6, true),
-	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_VCCAMS, AMS_VCCAMS, true),
-	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_VP_VN, AMS_VP_VN, false),
-	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY7, AMS_SUPPLY7, true),
-	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY8, AMS_SUPPLY8, true),
-	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY9, AMS_SUPPLY9, true),
-	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY10, AMS_SUPPLY10, true),
+	AMS_PL_CHAN_TEMP(AMS_SEQ_TEMP, AMS_TEMP, "Temp_PL"),
+	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY1, AMS_SUPPLY1, true, "VCCINT"),
+	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY2, AMS_SUPPLY2, true, "VCCAUX"),
+	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_VREFP, AMS_VREFP, false, "VREFP"),
+	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_VREFN, AMS_VREFN, false, "VREFN"),
+	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY3, AMS_SUPPLY3, true, "VCCBRAM"),
+	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY4, AMS_SUPPLY4, true, "VCC_PSINTLP"),
+	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY5, AMS_SUPPLY5, true, "VCC_PSINTFP"),
+	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY6, AMS_SUPPLY6, true, "VCC_PSAUX"),
+	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_VCCAMS, AMS_VCCAMS, true, "VCCAMS"),
+	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_VP_VN, AMS_VP_VN, false, "VP_VN"),
+	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY7, AMS_SUPPLY7, true, "VUser0"),
+	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY8, AMS_SUPPLY8, true, "VUser1"),
+	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY9, AMS_SUPPLY9, true, "VUser2"),
+	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY10, AMS_SUPPLY10, true, "VUser3"),
 	AMS_PL_AUX_CHAN_VOLTAGE(0),
 	AMS_PL_AUX_CHAN_VOLTAGE(1),
 	AMS_PL_AUX_CHAN_VOLTAGE(2),
@@ -1166,13 +1176,13 @@  static const struct iio_chan_spec ams_pl_channels[] = {
 };
 
 static const struct iio_chan_spec ams_ctrl_channels[] = {
-	AMS_CTRL_CHAN_VOLTAGE(AMS_SEQ_VCC_PSPLL, AMS_VCC_PSPLL0),
-	AMS_CTRL_CHAN_VOLTAGE(AMS_SEQ_VCC_PSBATT, AMS_VCC_PSPLL3),
-	AMS_CTRL_CHAN_VOLTAGE(AMS_SEQ_VCCINT, AMS_VCCINT),
-	AMS_CTRL_CHAN_VOLTAGE(AMS_SEQ_VCCBRAM, AMS_VCCBRAM),
-	AMS_CTRL_CHAN_VOLTAGE(AMS_SEQ_VCCAUX, AMS_VCCAUX),
-	AMS_CTRL_CHAN_VOLTAGE(AMS_SEQ_PSDDRPLL, AMS_PSDDRPLL),
-	AMS_CTRL_CHAN_VOLTAGE(AMS_SEQ_INTDDR, AMS_PSINTFPDDR),
+	AMS_CTRL_CHAN_VOLTAGE(AMS_SEQ_VCC_PSPLL, AMS_VCC_PSPLL0, "VCC_PSPLL"),
+	AMS_CTRL_CHAN_VOLTAGE(AMS_SEQ_VCC_PSBATT, AMS_VCC_PSPLL3, "VCC_PSBATT"),
+	AMS_CTRL_CHAN_VOLTAGE(AMS_SEQ_VCCINT, AMS_VCCINT, "VCCINT"),
+	AMS_CTRL_CHAN_VOLTAGE(AMS_SEQ_VCCBRAM, AMS_VCCBRAM, "VCCBRAM"),
+	AMS_CTRL_CHAN_VOLTAGE(AMS_SEQ_VCCAUX, AMS_VCCAUX, "VCCAUX"),
+	AMS_CTRL_CHAN_VOLTAGE(AMS_SEQ_PSDDRPLL, AMS_PSDDRPLL, "VCC_PSDDR_PLL"),
+	AMS_CTRL_CHAN_VOLTAGE(AMS_SEQ_INTDDR, AMS_PSINTFPDDR, "VCC_PSINTFP_DDR"),
 };
 
 static int ams_get_ext_chan(struct fwnode_handle *chan_node,
@@ -1336,6 +1346,7 @@  static int ams_parse_firmware(struct iio_dev *indio_dev)
 }
 
 static const struct iio_info iio_ams_info = {
+	.read_label = ams_read_label,
 	.read_raw = &ams_read_raw,
 	.read_event_config = &ams_read_event_config,
 	.write_event_config = &ams_write_event_config,