linux-kernel - Re: [PATCH RFC 05/13] dmaengine: sdxi: Add software data structures

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: <20250915125937.000072ab@huawei.com>
Date: Mon, 15 Sep 2025 12:59:37 +0100
From: Jonathan Cameron <jonathan.cameron@...wei.com>
To: Nathan Lynch via B4 Relay <devnull+nathan.lynch.amd.com@...nel.org>
CC: <nathan.lynch@....com>, Vinod Koul <vkoul@...nel.org>, Wei Huang
	<wei.huang2@....com>, Mario Limonciello <mario.limonciello@....com>, "Bjorn
 Helgaas" <bhelgaas@...gle.com>, <linux-pci@...r.kernel.org>,
	<linux-kernel@...r.kernel.org>, <dmaengine@...r.kernel.org>
Subject: Re: [PATCH RFC 05/13] dmaengine: sdxi: Add software data structures

On Fri, 05 Sep 2025 13:48:28 -0500
Nathan Lynch via B4 Relay <devnull+nathan.lynch.amd.com@...nel.org> wrote:

> From: Nathan Lynch <nathan.lynch@....com>
> 
> Add the driver's central header sdxi.h, which brings in the major
> software abstractions used throughout the driver -- mainly the SDXI
> device or function (sdxi_dev) and context (sdxi_cxt).
> 
> Co-developed-by: Wei Huang <wei.huang2@....com>
> Signed-off-by: Wei Huang <wei.huang2@....com>
> Signed-off-by: Nathan Lynch <nathan.lynch@....com>

I'm not personally a fan of 'header' patches.  It's find of reasonable if it's
just stuff of the datasheet, but once we get function definitions, we should
have the function implementations in the same patch.

> ---
>  drivers/dma/sdxi/sdxi.h | 206 ++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 206 insertions(+)
> 
> diff --git a/drivers/dma/sdxi/sdxi.h b/drivers/dma/sdxi/sdxi.h
> new file mode 100644
> index 0000000000000000000000000000000000000000..13e02f0541e0d60412c99b0b75bd37155a531e1d
> --- /dev/null
> +++ b/drivers/dma/sdxi/sdxi.h
> @@ -0,0 +1,206 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * SDXI device driver header
> + *
> + * Copyright (C) 2025 Advanced Micro Devices, Inc.
> + */
> +
> +#ifndef __SDXI_H
> +#define __SDXI_H
> +
> +#include <linux/dev_printk.h>
> +#include <linux/device.h>
> +#include <linux/dma-mapping.h>
> +#include <linux/dmapool.h>

Some of these could I think be removed in favor of one or two forwards
definitions.  In general good to keep to minimal includes following principles of
include what you use din each file.

> +#include <linux/dmaengine.h>
> +#include <linux/io-64-nonatomic-lo-hi.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/types.h>



> +/* Device Control */

Superficially these don't seem have anything to do with controlling
the device. So this comment is confusing to me rather than helpful.

> +int sdxi_device_init(struct sdxi_dev *sdxi, const struct sdxi_dev_ops *ops);
> +void sdxi_device_exit(struct sdxi_dev *sdxi);

Bring these in with the code, not in an earlier patch.
Ideally set things up so the code is build able after each patch.