linux-kernel - Re: [PATCH v4 06/10] memory: ti-aemif: Export aemif_*_cs

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]

Message-ID: <3a74b938-072e-443c-bf48-d97aff969a17@kernel.org>
Date: Thu, 21 Nov 2024 11:33:49 +0100
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Bastien Curutchet <bastien.curutchet@...tlin.com>,
 Santosh Shilimkar <ssantosh@...nel.org>,
 Miquel Raynal <miquel.raynal@...tlin.com>,
 Richard Weinberger <richard@....at>, Vignesh Raghavendra <vigneshr@...com>
Cc: linux-kernel@...r.kernel.org, linux-mtd@...ts.infradead.org,
 Thomas Petazzoni <thomas.petazzoni@...tlin.com>,
 Herve Codina <herve.codina@...tlin.com>,
 Christopher Cordahi <christophercordahi@...ometrics.ca>
Subject: Re: [PATCH v4 06/10] memory: ti-aemif: Export aemif_*_cs_timings()

On 15/11/2024 14:26, Bastien Curutchet wrote:
>  	return 0;
>  }
> +EXPORT_SYMBOL(aemif_check_cs_timings);
>  
>  /**
>   * aemif_set_cs_timings - Set the timing configuration of a given chip select.
> @@ -173,7 +158,7 @@ static int aemif_check_cs_timings(struct aemif_cs_timings *timings)
>   *
>   * @return: 0 on success, else negative errno.
>   */
> -static int aemif_set_cs_timings(struct aemif_device *aemif, u8 cs, struct aemif_cs_timings *timings)
> +int aemif_set_cs_timings(struct aemif_device *aemif, u8 cs, struct aemif_cs_timings *timings)
>  {
>  	unsigned int offset;
>  	u32 val, set;
> @@ -195,13 +180,16 @@ static int aemif_set_cs_timings(struct aemif_device *aemif, u8 cs, struct aemif_
>  
>  	offset = A1CR_OFFSET + cs * 4;
>  
> +	mutex_lock(&aemif->config_cs_lock);
>  	val = readl(aemif->base + offset);
>  	val &= ~TIMINGS_MASK;
>  	val |= set;
>  	writel(val, aemif->base + offset);
> +	mutex_unlock(&aemif->config_cs_lock);
>  
>  	return 0;
>  }
> +EXPORT_SYMBOL(aemif_set_cs_timings);


EXPORT_SYMBOL_GPL everywhere, these are quite specific to driver's
internals, so internal implementation.

Also, all of exported functions need to have correct kerneldoc but I
think they don't. At least missing(), so maybe rest also did not conform
to kernel doc style.

>  
>  /**
>   * aemif_calc_rate - calculate timing data.
> @@ -257,10 +245,12 @@ static int aemif_config_abus(struct platform_device *pdev, int csnum)
>  	if (data->enable_ss)
>  		set |= ACR_SSTROBE_MASK;
>  
> +	mutex_lock(&aemif->config_cs_lock);
>  	val = readl(aemif->base + offset);
>  	val &= ~CONFIG_MASK;
>  	val |= set;
>  	writel(val, aemif->base + offset);
> +	mutex_unlock(&aemif->config_cs_lock);
>  
>  	return aemif_set_cs_timings(aemif, data->cs - aemif->cs_offset, &data->timings);
>  }
> @@ -399,6 +389,7 @@ static int aemif_probe(struct platform_device *pdev)
>  	if (IS_ERR(aemif->base))
>  		return PTR_ERR(aemif->base);
>  
> +	mutex_init(&aemif->config_cs_lock);
>  	if (np) {
>  		/*
>  		 * For every controller device node, there is a cs device node
> diff --git a/include/linux/memory/ti-aemif.h b/include/linux/memory/ti-aemif.h
> new file mode 100644
> index 000000000000..0640d30f6321
> --- /dev/null
> +++ b/include/linux/memory/ti-aemif.h
> @@ -0,0 +1,32 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +#ifndef __TI_AEMIF_H
> +#define __TI_AEMIF_H


Use some longer header guard, e.g. __MEMORY_TI_AEMIF_H

> +
> +/**



Best regards,
Krzysztof