linux-kernel - Re: [RFC PATCH 01/10] dma-engine: sun4i: Add a quirk to support different chips

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] [day] [month] [year] [list]

Date:   Fri, 4 Jan 2019 21:08:15 +0530
From:   Vinod Koul <vkoul@...nel.org>
To:     Mesih Kilinc <mesihkilinc@...il.com>
Cc:     dmaengine@...r.kernel.org, devicetree@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
        alsa-devel@...a-project.org, linux-sunxi@...glegroups.com,
        Rob Herring <robh+dt@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        Maxime Ripard <maxime.ripard@...tlin.com>,
        Chen-Yu Tsai <wens@...e.org>,
        Liam Girdwood <lgirdwood@...il.com>,
        Mark Brown <broonie@...nel.org>,
        Jaroslav Kysela <perex@...ex.cz>, Takashi Iwai <tiwai@...e.com>
Subject: Re: [RFC PATCH 01/10] dma-engine: sun4i: Add a quirk to support
 different chips

On 03-12-18, 00:23, Mesih Kilinc wrote:
> Allwinner suniv F1C100s has similar DMA engine to sun4i. Several
> registers has different addresses. Total dma channels, endpoint counts
> and max burst counts are also different.

So if register layout is the only diff, have you thought about using
regmap_field so that you dont care about the layout

> +struct sun4i_dma_config {
> +	u32 ndma_nr_max_channels;
> +	u32 ndma_nr_max_vchans;
> +
> +	u32 ddma_nr_max_channels;
> +	u32 ddma_nr_max_vchans;
> +
> +	u32 dma_nr_max_channels;
> +
> +        void (*set_dst_data_width)(u32 *p_cfg, s8 data_width);
> +        void (*set_src_data_width)(u32 *p_cfg, s8 data_width);

aligned please, checkpatch should warn about this stuff

>  struct sun4i_dma_dev {
> -	DECLARE_BITMAP(pchans_used, SUN4I_DMA_NR_MAX_CHANNELS);
> +	unsigned long *pchans_used;

why not bitmap?

> +static struct sun4i_dma_config sun4i_a10_dma_cfg = {
> +	.ndma_nr_max_channels	= SUN4I_NDMA_NR_MAX_CHANNELS,
> +	.ndma_nr_max_vchans	= SUN4I_NDMA_NR_MAX_VCHANS,
> +
> +	.ddma_nr_max_channels	= SUN4I_DDMA_NR_MAX_CHANNELS,
> +	.ddma_nr_max_vchans	= SUN4I_DDMA_NR_MAX_VCHANS,
> +
> +	.dma_nr_max_channels	= SUN4I_NDMA_NR_MAX_CHANNELS + 
> +		SUN4I_DDMA_NR_MAX_CHANNELS,
> +
> +	.set_dst_data_width	= set_dst_data_width_a10,
> +	.set_src_data_width	= set_src_data_width_a10,
> +	.convert_burst		= convert_burst_a10,
> +
> +	.ndma_drq_sdram		= SUN4I_NDMA_DRQ_TYPE_SDRAM,
> +	.ddma_drq_sdram		= SUN4I_DDMA_DRQ_TYPE_SDRAM,
> +
> +	.max_burst		= SUN4I_MAX_BURST,
> +};
> +
>  static const struct of_device_id sun4i_dma_match[] = {
> -	{ .compatible = "allwinner,sun4i-a10-dma" },
> +	{ .compatible = "allwinner,sun4i-a10-dma", .data = &sun4i_a10_dma_cfg },

I would prefer this to be split into two patches, first one does the
refactoring of driver (better if we split things logically) and then
addition of new one..

-- 
~Vinod