lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Tue, 28 Mar 2023 16:58:26 +0530
From:   Md Danish Anwar <a0501179@...com>
To:     Mathieu Poirier <mathieu.poirier@...aro.org>,
        MD Danish Anwar <danishanwar@...com>
CC:     "Andrew F. Davis" <afd@...com>, Suman Anna <s-anna@...com>,
        Roger Quadros <rogerq@...nel.org>,
        Vignesh Raghavendra <vigneshr@...com>,
        Tero Kristo <kristo@...nel.org>,
        Bjorn Andersson <andersson@...nel.org>,
        Santosh Shilimkar <ssantosh@...nel.org>,
        Nishanth Menon <nm@...com>, <linux-remoteproc@...r.kernel.org>,
        <linux-arm-kernel@...ts.infradead.org>,
        <linux-kernel@...r.kernel.org>, <linux-omap@...r.kernel.org>,
        <srk@...com>, <devicetree@...r.kernel.org>,
        <netdev@...r.kernel.org>
Subject: Re: [EXTERNAL] Re: [PATCH v5 5/5] soc: ti: pruss: Add helper
 functions to get/set PRUSS_CFG_GPMUX



On 28/03/23 02:34, Mathieu Poirier wrote:
> On Thu, Mar 23, 2023 at 11:54:51AM +0530, MD Danish Anwar wrote:
>> From: Tero Kristo <t-kristo@...com>
>>
>> Add two new helper functions pruss_cfg_get_gpmux() & pruss_cfg_set_gpmux()
>> to get and set the GP MUX mode for programming the PRUSS internal wrapper
>> mux functionality as needed by usecases.
>>
>> Co-developed-by: Suman Anna <s-anna@...com>
>> Signed-off-by: Suman Anna <s-anna@...com>
>> Signed-off-by: Tero Kristo <t-kristo@...com>
>> Co-developed-by: Grzegorz Jaszczyk <grzegorz.jaszczyk@...aro.org>
>> Signed-off-by: Grzegorz Jaszczyk <grzegorz.jaszczyk@...aro.org>
>> Signed-off-by: Puranjay Mohan <p-mohan@...com>
>> Signed-off-by: MD Danish Anwar <danishanwar@...com>
>> Reviewed-by: Roger Quadros <rogerq@...nel.org>
>> ---
>>  drivers/soc/ti/pruss.c           | 44 ++++++++++++++++++++++++++++++++
>>  include/linux/remoteproc/pruss.h | 30 ++++++++++++++++++++++
>>  2 files changed, 74 insertions(+)
>>
>> diff --git a/drivers/soc/ti/pruss.c b/drivers/soc/ti/pruss.c
>> index ac415442e85b..3aa3c38c6c79 100644
>> --- a/drivers/soc/ti/pruss.c
>> +++ b/drivers/soc/ti/pruss.c
>> @@ -239,6 +239,50 @@ int pruss_cfg_xfr_enable(struct pruss *pruss, enum pru_type pru_type,
>>  }
>>  EXPORT_SYMBOL_GPL(pruss_cfg_xfr_enable);
>>  
>> +/**
>> + * pruss_cfg_get_gpmux() - get the current GPMUX value for a PRU device
>> + * @pruss: pruss instance
>> + * @pru_id: PRU identifier (0-1)
>> + * @mux: pointer to store the current mux value into
>> + *
>> + * Return: 0 on success, or an error code otherwise
>> + */
>> +int pruss_cfg_get_gpmux(struct pruss *pruss, enum pruss_pru_id pru_id, u8 *mux)
>> +{
>> +	int ret = 0;
>> +	u32 val;
>> +
>> +	if (pru_id < 0 || pru_id >= PRUSS_NUM_PRUS)
>> +		return -EINVAL;
>> +
>> +	ret = pruss_cfg_read(pruss, PRUSS_CFG_GPCFG(pru_id), &val);
>> +	if (!ret)
>> +		*mux = (u8)((val & PRUSS_GPCFG_PRU_MUX_SEL_MASK) >>
>> +			    PRUSS_GPCFG_PRU_MUX_SEL_SHIFT);
> 
> What happens if @mux is NULL?

@mux being null may result in some error here. I will add NULL check for mux
before storing the value in mux.

I will modify the above if condition to have NULL check for mux as well.
The if condition will look like below.

	if (pru_id < 0 || pru_id >= PRUSS_NUM_PRUS || !mux)
		return -EINVAL;

Please let me know if this looks OK.

> 
> Thanks,
> Mathieu
> 
> 
>> +	return ret;
>> +}
>> +EXPORT_SYMBOL_GPL(pruss_cfg_get_gpmux);
>> +
>> +/**
>> + * pruss_cfg_set_gpmux() - set the GPMUX value for a PRU device
>> + * @pruss: pruss instance
>> + * @pru_id: PRU identifier (0-1)
>> + * @mux: new mux value for PRU
>> + *
>> + * Return: 0 on success, or an error code otherwise
>> + */
>> +int pruss_cfg_set_gpmux(struct pruss *pruss, enum pruss_pru_id pru_id, u8 mux)
>> +{
>> +	if (mux >= PRUSS_GP_MUX_SEL_MAX ||
>> +	    pru_id < 0 || pru_id >= PRUSS_NUM_PRUS)
>> +		return -EINVAL;
>> +
>> +	return pruss_cfg_update(pruss, PRUSS_CFG_GPCFG(pru_id),
>> +				PRUSS_GPCFG_PRU_MUX_SEL_MASK,
>> +				(u32)mux << PRUSS_GPCFG_PRU_MUX_SEL_SHIFT);
>> +}
>> +EXPORT_SYMBOL_GPL(pruss_cfg_set_gpmux);
>> +
>>  static void pruss_of_free_clk_provider(void *data)
>>  {
>>  	struct device_node *clk_mux_np = data;
>> diff --git a/include/linux/remoteproc/pruss.h b/include/linux/remoteproc/pruss.h
>> index bb001f712980..42f1586c62ac 100644
>> --- a/include/linux/remoteproc/pruss.h
>> +++ b/include/linux/remoteproc/pruss.h
>> @@ -16,6 +16,24 @@
>>  
>>  #define PRU_RPROC_DRVNAME "pru-rproc"
>>  
>> +/*
>> + * enum pruss_gp_mux_sel - PRUSS GPI/O Mux modes for the
>> + * PRUSS_GPCFG0/1 registers
>> + *
>> + * NOTE: The below defines are the most common values, but there
>> + * are some exceptions like on 66AK2G, where the RESERVED and MII2
>> + * values are interchanged. Also, this bit-field does not exist on
>> + * AM335x SoCs
>> + */
>> +enum pruss_gp_mux_sel {
>> +	PRUSS_GP_MUX_SEL_GP = 0,
>> +	PRUSS_GP_MUX_SEL_ENDAT,
>> +	PRUSS_GP_MUX_SEL_RESERVED,
>> +	PRUSS_GP_MUX_SEL_SD,
>> +	PRUSS_GP_MUX_SEL_MII2,
>> +	PRUSS_GP_MUX_SEL_MAX,
>> +};
>> +
>>  /*
>>   * enum pruss_gpi_mode - PRUSS GPI configuration modes, used
>>   *			 to program the PRUSS_GPCFG0/1 registers
>> @@ -110,6 +128,8 @@ int pruss_cfg_gpimode(struct pruss *pruss, enum pruss_pru_id pru_id,
>>  int pruss_cfg_miirt_enable(struct pruss *pruss, bool enable);
>>  int pruss_cfg_xfr_enable(struct pruss *pruss, enum pru_type pru_type,
>>  			 bool enable);
>> +int pruss_cfg_get_gpmux(struct pruss *pruss, enum pruss_pru_id pru_id, u8 *mux);
>> +int pruss_cfg_set_gpmux(struct pruss *pruss, enum pruss_pru_id pru_id, u8 mux);
>>  
>>  #else
>>  
>> @@ -152,6 +172,16 @@ static inline int pruss_cfg_xfr_enable(struct pruss *pruss,
>>  	return ERR_PTR(-EOPNOTSUPP);
>>  }
>>  
>> +static inline int pruss_cfg_get_gpmux(struct pruss *pruss, enum pruss_pru_id pru_id, u8 *mux)
>> +{
>> +	return ERR_PTR(-EOPNOTSUPP);
>> +}
>> +
>> +static inline int pruss_cfg_set_gpmux(struct pruss *pruss, enum pruss_pru_id pru_id, u8 mux)
>> +{
>> +	return ERR_PTR(-EOPNOTSUPP);
>> +}
>> +
>>  #endif /* CONFIG_TI_PRUSS */
>>  
>>  #if IS_ENABLED(CONFIG_PRU_REMOTEPROC)
>> -- 
>> 2.25.1
>>

-- 
Thanks and Regards,
Danish.

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ