linux-kernel - Re: [PATCH v2 5/8] mmc: sdhci: add Black Sesame Technologies BST C1200 controller driver

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <2559f035-787e-4c80-8889-d1826a27171b@kernel.org>
Date: Wed, 2 Jul 2025 12:47:40 +0200
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Albert Yang <yangzh0906@...ndersoft.com>, robh@...nel.org,
 krzk+dt@...nel.org, conor+dt@...nel.org, gordon.ge@....ai,
 catalin.marinas@....com, geert.uytterhoeven@...il.com, will@...nel.org,
 ulf.hansson@...aro.org, adrian.hunter@...el.com, arnd@...db.de
Cc: linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
 devicetree@...r.kernel.org, linux-mmc@...r.kernel.org, soc@...ts.linux.dev,
 bst-upstream@...ai.top, neil.armstrong@...aro.org,
 jonathan.cameron@...wei.com, bigfoot@...ssfun.cn, kever.yang@...k-chips.com,
 mani@...nel.org, geert+renesas@...der.be, andersson@...nel.org, nm@...com,
 nfraprado@...labora.com, quic_tdas@...cinc.com, ebiggers@...gle.com,
 victor.shih@...esyslogic.com.tw, shanchun1218@...il.com,
 ben.chuang@...esyslogic.com.tw
Subject: Re: [PATCH v2 5/8] mmc: sdhci: add Black Sesame Technologies BST
 C1200 controller driver

On 02/07/2025 11:44, Albert Yang wrote:
> Add a driver for the DesignWare Mobile Storage Host Controller (DWCMSHC)
> SDHCI controller found in Black Sesame Technologies C1200 SoCs.
> 
> The driver provides specialized clock configuration, tuning, voltage
> switching, and power management for the BST DWCMSHC controller. It also
> includes support for eMMC boot and memory-mapped I/O for CRM registers.
> 

Missing SoB.

...

> +
> +static int bst_sdhci_reallocate_bounce_buffer(struct sdhci_host *host)
> +{
> +	struct mmc_host *mmc = host->mmc;
> +	unsigned int max_blocks;
> +	unsigned int bounce_size;
> +	int ret;
> +
> +	/*
> +	 * Cap the bounce buffer at 64KB. Using a bigger bounce buffer
> +	 * has diminishing returns, this is probably because SD/MMC
> +	 * cards are usually optimized to handle this size of requests.
> +	 */
> +	bounce_size = SZ_32K;
> +	/*
> +	 * Adjust downwards to maximum request size if this is less
> +	 * than our segment size, else hammer down the maximum
> +	 * request size to the maximum buffer size.
> +	 */
> +	if (mmc->max_req_size < bounce_size)
> +		bounce_size = mmc->max_req_size;
> +	max_blocks = bounce_size / 512;
> +
> +	ret = of_reserved_mem_device_init_by_idx(mmc_dev(mmc), mmc_dev(mmc)->of_node, 0);
> +	if (ret) {
> +		dev_err(mmc_dev(mmc), "Failed to initialize reserved memory\n");
> +		return ret;
> +	}
> +
> +	host->bounce_buffer = dma_alloc_coherent(mmc_dev(mmc), bounce_size,
> +						 &host->bounce_addr, GFP_KERNEL);
> +	if (!host->bounce_buffer)
> +		return -ENOMEM;
> +
> +	host->bounce_buffer_size = bounce_size;
> +
> +	/* Lie about this since we're bouncing */
> +	mmc->max_segs = max_blocks;
> +	mmc->max_seg_size = bounce_size;
> +	mmc->max_req_size = bounce_size;
> +
> +	dev_info(mmc_dev(mmc), "BST reallocate %s bounce up to %u segments into one, max segment size %u bytes\n",
> +		 mmc_hostname(mmc), max_blocks, bounce_size);

Devices are supposed to be silent on success.

> +


...

> +/**
> + * dwcmshc_remove - Platform driver remove
> + * @pdev: Platform device
> + *
> + * Removes the SDHCI host controller.
> + *
> + * Return: 0 on success
> + */
Drop all such fake comments, not helpful. We all now what is the purpose
of the function and saying that platform driver remove callback is
"platform driver remove" which "Removes the SDHCI host controller." is
not only redundant, but actually harming because later you have:
"Return: 0 on success"
which is impossible.

Such redundant comments are not kernel coding style. Provide USEFUL
comments, useful kerneldoc, not something to satisfy line-counters.


Best regards,
Krzysztof