On 29/03/23 23:34, Mathieu Poirier wrote:
> On Tue, 28 Mar 2023 at 05:28, Md Danish Anwar <a0501179@xxxxxx> wrote:
>>
>> 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@xxxxxx>
>>>>
>>>> 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@xxxxxx>
>>>> Signed-off-by: Suman Anna <s-anna@xxxxxx>
>>>> Signed-off-by: Tero Kristo <t-kristo@xxxxxx>
>>>> Co-developed-by: Grzegorz Jaszczyk <grzegorz.jaszczyk@xxxxxxxxxx>
>>>> Signed-off-by: Grzegorz Jaszczyk <grzegorz.jaszczyk@xxxxxxxxxx>
>>>> Signed-off-by: Puranjay Mohan <p-mohan@xxxxxx>
>>>> Signed-off-by: MD Danish Anwar <danishanwar@xxxxxx>
>>>> Reviewed-by: Roger Quadros <rogerq@xxxxxxxxxx>
>>>> ---
>>>> 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.
>>
>
> It will result in a kernel panic.
>
>> 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;
>>
>
> That will be fine.>
Sure, I'll go ahead and make this change.
>> 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.
--
Thanks and Regards,
Danish.
