Re: [PATCH 06/10] mci: core: use CONFIG_MCI_WRITE, not CONFIG_BLOCK_WRITE

[Ahmad Fatoum <a.fatoum@pengutronix.de>]

Hello again,

On 30.07.24 13:08, Ahmad Fatoum wrote:
> Hello Sascha,
> 
> On 30.07.24 11:18, Sascha Hauer wrote:
>> On Tue, Jul 30, 2024 at 09:19:25AM +0200, Ahmad Fatoum wrote:
>>> There's a more specific CONFIG_MCI_WRITE that's so far only used to
>>> remove write support for in the Atmel MCI driver. We should use the same
>>> symbol also to remove support in the MCI core instead of relying on its
>>> parent CONFIG_BLOCK_WRITE option.
>>
>> Currently CONFIG_MCI_WRITE has no relation to CONFIG_BLOCK_WRITE.
>> Having CONFIG_MCI_WRITE enabled and CONFIG_BLOCK_WRITE disabled doesn't
>> make sense. Shouldn't CONFIG_MCI_WRITE depend on CONFIG_BLOCK_WRITE?
>>
>> Also having CONFIG_BLOCK_WRITE enabled and CONFIG_MCI_WRITE disabled
>> allows you to support writing to block devices that are not MCI devices.
>> Given that the vast majority of block devices are actually MCI devices
>> this seems rather exotic.
>>
>> Maybe we should drop CONFIG_MCI_WRITE and replace it with
>> CONFIG_BLOCK_WRITE, or just have
>>
>> config MCI_WRITE
>> 	bool
>> 	default y if BLOCK_WRITE
> 
> This works for me. I can do this for v2.

I looked into this and MCI_WRITE selects DISK_WRITE, which selects
BLOCK_WRITE. BLOCK_WRITE has no prompt. I'd prefer to keep this as-is.

> 
> 
>>
>> Sascha
>>
>>>
>>> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
>>> ---
>>>  drivers/mci/mci-core.c | 8 +++-----
>>>  1 file changed, 3 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/mci/mci-core.c b/drivers/mci/mci-core.c
>>> index f6f8a6adabb9..3a5fb0330700 100644
>>> --- a/drivers/mci/mci-core.c
>>> +++ b/drivers/mci/mci-core.c
>>> @@ -1801,8 +1801,8 @@ static int mci_blk_part_switch(struct mci_part *part)
>>>   *
>>>   * This routine expects the buffer has the correct size to read all data!
>>>   */
>>> -static int __maybe_unused mci_sd_write(struct block_device *blk,
>>> -				const void *buffer, sector_t block, blkcnt_t num_blocks)
>>> +static int mci_sd_write(struct block_device *blk,
>>> +			const void *buffer, sector_t block, blkcnt_t num_blocks)
>>>  {
>>>  	struct mci_part *part = container_of(blk, struct mci_part, blk);
>>>  	struct mci *mci = part->mci;
>>> @@ -2179,9 +2179,7 @@ static int mci_check_if_already_initialized(struct mci *mci)
>>>  
>>>  static struct block_device_ops mci_ops = {
>>>  	.read = mci_sd_read,
>>> -#ifdef CONFIG_BLOCK_WRITE
>>> -	.write = mci_sd_write,
>>> -#endif
>>> +	.write = IS_ENABLED(CONFIG_MCI_WRITE) ? mci_sd_write : NULL,
>>>  };
>>>  
>>>  static int mci_set_boot(struct param_d *param, void *priv)
>>> -- 
>>> 2.39.2
>>>
>>>
>>>
>>
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |