Message-ID: <f9138082-1d17-4247-b365-7cb4eef88879@kernel.org>
Date: Sat, 28 Dec 2024 12:02:43 +0100
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Qunqin Zhao <zhaoqunqin@...ngson.cn>, arnd@...db.de, olof@...om.net,
 soc@...nel.org
Cc: soc@...ts.linux.dev, linux-kernel@...r.kernel.org,
 loongarch@...ts.linux.dev, xry111@...111.site, christophe.jaillet@...adoo.fr
Subject: Re: [PATCH V2 RESEND] soc: loongson: add Loongson Security Module
 driver

On 24/12/2024 08:25, Qunqin Zhao wrote:
> This driver supports Loongson Security Module, which
> provides the control for it's hardware encryption
> acceleration child devices.

Please wrap commit message according to Linux coding style / submission
process (neither too early nor over the limit):
https://elixir.bootlin.com/linux/v6.4-rc1/source/Documentation/process/submitting-patches.rst#L597

> 
> Only ACPI firmware is supported.
> 
> Signed-off-by: Qunqin Zhao <zhaoqunqin@...ngson.cn>

I don't get why you are sending this to soc. This is loongson specific
and is supposed to go via Loongson maintainers.

And why is this a resend?

> ---
> Changes in v2:
> 	- Removed useless memset() in probe.
> 	- Cleaned up coding style.
> 	- Corrected the spelling.
> 
>  MAINTAINERS                        |   7 +
>  drivers/soc/loongson/Kconfig       |   9 +
>  drivers/soc/loongson/Makefile      |   1 +
>  drivers/soc/loongson/loongson_se.c | 523 +++++++++++++++++++++++++++++
>  include/soc/loongson/se.h          | 135 ++++++++


include/linux/soc/... or just keep it private to the driver. Why this
has to be exposed to other users?

>  5 files changed, 675 insertions(+)
>  create mode 100644 drivers/soc/loongson/loongson_se.c
>  create mode 100644 include/soc/loongson/se.h
> 

...

> +
> +/*
> + * Called by SE's child device driver.
> + */
> +int se_send_ch_requeset(struct lsse_ch *ch)
> +{
> +	struct loongson_se *se;
> +	u32 status, int_bit;
> +
> +	se = ch->se;
> +	int_bit = ch->int_bit;
> +	if ((se_readl(se, SE_L2SINT_STAT) & int_bit) ||
> +	    !(se_readl(se, SE_L2SINT_EN) & int_bit))
> +		return -EBUSY;
> +
> +	se_enable_int(se, int_bit);
> +	se_writel(se, int_bit, SE_L2SINT_SET);
> +
> +	return readl_relaxed_poll_timeout_atomic(se->base + SE_L2SINT_STAT, status,
> +						 !(status & int_bit), 10, 10000);
> +
> +}
> +EXPORT_SYMBOL_GPL(se_send_ch_requeset);

No, no users. You cannot export unused symbols.

> +
> +/*
> + * se_init_ch() - Init the channel used by child device.
> + *
> + * Allocate the shared memory agreed upon with SE on SE probe,
> + * and register the callback function when the data processing
> + * in this channel is completed.
> + */
> +struct lsse_ch *se_init_ch(struct device *dev, int id, int data_size, int msg_size,
> +			   void *priv, void (*complete)(struct lsse_ch *se_ch))
> +{
> +	struct loongson_se *se = dev_get_drvdata(dev);
> +	struct lsse_ch *ch;
> +	unsigned long flag;
> +	int data_first, data_nr;
> +	int msg_first, msg_nr;
> +
> +	if (!se) {
> +		pr_err("SE has bot been initialized\n");
> +		return NULL;
> +	}
> +
> +	if (id > SE_CH_MAX) {
> +		dev_err(se->dev, "Channel number %d is invalid\n", id);
> +		return NULL;
> +	}
> +
> +	if (se_ch_status(se, BIT(id))) {
> +		dev_err(se->dev, "Channel number %d has been initialized\n", id);
> +		return NULL;
> +	}
> +
> +	spin_lock_irqsave(&se->dev_lock, flag);
> +
> +	ch = &se->chs[id];
> +	ch->se = se;
> +	ch->id = id;
> +	ch->int_bit = BIT(id);
> +	se->ch_status |= BIT(id);
> +
> +	data_nr = round_up(data_size, PAGE_SIZE) / PAGE_SIZE;
> +	data_first = bitmap_find_next_zero_area(se->mem_map, se->mem_map_pages,
> +						0, data_nr, 0);
> +	if (data_first >= se->mem_map_pages) {
> +		dev_err(se->dev, "Insufficient memory space\n");
> +		spin_unlock_irqrestore(&se->dev_lock, flag);
> +		return NULL;
> +	}
> +
> +	bitmap_set(se->mem_map, data_first, data_nr);
> +	ch->data_buffer = se->mem_base + data_first * PAGE_SIZE;
> +	ch->data_addr = se->mem_addr + data_first * PAGE_SIZE;
> +	ch->data_size = data_size;
> +
> +	msg_nr = round_up(msg_size, PAGE_SIZE) / PAGE_SIZE;
> +	msg_first = bitmap_find_next_zero_area(se->mem_map, se->mem_map_pages,
> +					       0, msg_nr, 0);
> +	if (msg_first >= se->mem_map_pages) {
> +		dev_err(se->dev, "Insufficient memory space\n");
> +		bitmap_clear(se->mem_map, data_first, data_nr);
> +		spin_unlock_irqrestore(&se->dev_lock, flag);
> +		return NULL;
> +	}
> +
> +	bitmap_set(se->mem_map, msg_first, msg_nr);
> +	ch->smsg = se->mem_base + msg_first * PAGE_SIZE;
> +	ch->rmsg = ch->smsg + msg_size / 2;
> +	ch->msg_size = msg_size;
> +	ch->complete = complete;
> +	ch->priv = priv;
> +	spin_lock_init(&ch->ch_lock);
> +
> +	spin_unlock_irqrestore(&se->dev_lock, flag);
> +
> +	if (loongson_se_set_msg(ch)) {
> +		dev_err(se->dev, "Channel %d setup message address failed\n", id);
> +		return NULL;
> +	}
> +
> +	se_enable_int(se, ch->int_bit);
> +
> +	return ch;
> +}
> +EXPORT_SYMBOL_GPL(se_init_ch);

No, no users. You cannot export unused symbols.

> +
> +void se_deinit_ch(struct lsse_ch *ch)
> +{
> +	struct loongson_se *se = ch->se;
> +	unsigned long flag;
> +	int first, nr;
> +	int id = ch->id;
> +
> +	if (!se) {
> +		pr_err("SE has bot been initialized\n");
> +		return;
> +	}
> +
> +	if (id > SE_CH_MAX) {
> +		dev_err(se->dev, "Channel number %d is invalid\n", id);
> +		return;
> +	}
> +
> +	if (!se_ch_status(se, BIT(id))) {
> +		dev_err(se->dev, "Channel number %d has not been initialized\n", id);
> +		return;
> +	}
> +
> +	spin_lock_irqsave(&se->dev_lock, flag);
> +	se->ch_status &= ~BIT(ch->id);
> +
> +	first = (ch->data_buffer - se->mem_base) / PAGE_SIZE;
> +	nr = round_up(ch->data_size, PAGE_SIZE) / PAGE_SIZE;
> +	bitmap_clear(se->mem_map, first, nr);
> +
> +	first = (ch->smsg - se->mem_base) / PAGE_SIZE;
> +	nr = round_up(ch->msg_size, PAGE_SIZE) / PAGE_SIZE;
> +	bitmap_clear(se->mem_map, first, nr);
> +
> +	se_disable_int(se, ch->int_bit);
> +	spin_unlock_irqrestore(&se->dev_lock, flag);
> +
> +}
> +EXPORT_SYMBOL_GPL(se_deinit_ch);

No, no users. You cannot export unused symbols.

Best regards,
Krzysztof

Powered by blists - more mailing lists