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

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

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.


> 
> 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 |