From mboxrd@z Thu Jan 1 00:00:00 1970
From: Pratyush Yadav
Date: Tue, 19 Jan 2021 17:36:00 +0530
Subject: [PATCH 1/3] mem: spi-mem: define spi_mem_default_supports_op
In-Reply-To: <20210118235256.29748-2-matt@traverse.com.au>
References: <20210118235256.29748-1-matt@traverse.com.au>
<20210118235256.29748-2-matt@traverse.com.au>
Message-ID: <20210119120558.ga3trinnpd6hxbcm@ti.com>
List-Id:
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de
Hi Matthew,
> Subject: [PATCH 1/3] mem: spi-mem: define spi_mem_default_supports_op
Nitpick: You are declaring spi_mem_default_supports_op() here. It is
already defined.
On 18/01/21 11:52PM, Mathew McBride wrote:
> spi_mem_default_supports_op is used internally by controller
> drivers to verify operation semantics are correct.
>
> Signed-off-by: Mathew McBride
> ---
> include/spi-mem.h | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/include/spi-mem.h b/include/spi-mem.h
> index ca0f55c8fd..92c640dabe 100644
> --- a/include/spi-mem.h
> +++ b/include/spi-mem.h
> @@ -216,6 +216,10 @@ int spi_controller_dma_map_mem_op_data(struct spi_controller *ctlr,
> void spi_controller_dma_unmap_mem_op_data(struct spi_controller *ctlr,
> const struct spi_mem_op *op,
> struct sg_table *sg);
> +
> +bool spi_mem_default_supports_op(struct spi_mem *mem,
> + const struct spi_mem_op *op);
> +
This block of code was imported verbatim from the Linux driver and then
wrapped around with a #ifndef __UBOOT__ to avoid compilation errors. So
it will never get "enabled" in U-Boot ever. No driver can use the
prototypes you have added.
And I tested this by applying your patch series and building the
fsl_qspi driver using ls1012aqds_qspi_defconfig. Sure enough, the
compiler reported "implicit declaration of function
spi_mem_default_supports_op". Strangely, the linker did not complain and
went through without errors. Not sure which function it would end up
linking there.
Move the declaration outside this ifdef, right beside where
spi_mem_supports_op() is declared. No need to have the variant below. It
is safe to assume CONFIG_SPI_MEM is enabled if spi-mem.h is included.
> #else
> static inline int
> spi_controller_dma_map_mem_op_data(struct spi_controller *ctlr,
> @@ -231,6 +235,12 @@ spi_controller_dma_unmap_mem_op_data(struct spi_controller *ctlr,
> struct sg_table *sg)
> {
> }
> +
> +bool spi_mem_default_supports_op(struct spi_mem *mem,
> + const struct spi_mem_op *op)
> +{
> + return false;
> +}
> #endif /* CONFIG_SPI_MEM */
> #endif /* __UBOOT__ */
>
> --
> 2.30.0
>
--
Regards,
Pratyush Yadav
Texas Instruments India