(I moved Lorenzo's comment to better see the macro)
Lorenzo Bianconi <lorenzo@xxxxxxxxxx> writes:
>> From: Peter Chiu <chui-hao.chiu@xxxxxxxxxxxx>
>>
>> Add DSP firmware for phy-related control. The firmware is transparent to
>> driver, but it's necessary for the firmware download process.
>>
>> Reviewed-by: Shayne Chen <shayne.chen@xxxxxxxxxxxx>
>> Signed-off-by: Peter Chiu <chui-hao.chiu@xxxxxxxxxxxx>
>> Signed-off-by: Shayne Chen <shayne.chen@xxxxxxxxxxxx>
[...]
>> +#define LOAD_RAM(_type) \
>> + do { \
>> + ret = request_firmware(&fw, MT7996_FIRMWARE_##_type, dev->mt76.dev);\
>> + if (ret) \
>> + return ret; \
>> + \
>> + if (!fw || !fw->data || fw->size < sizeof(*hdr)) { \
>> + dev_err(dev->mt76.dev, "Invalid firmware\n"); \
>> + release_firmware(fw); \
>> + return -EINVAL; \
>> + } \
>> + \
>> + hdr = (const struct mt7996_fw_trailer *) \
>> + (fw->data + fw->size - sizeof(*hdr)); \
>> + \
>> + dev_info(dev->mt76.dev, \
>> + "%s Firmware Version: %.10s, Build Time: %.15s\n", \
>> + #_type, hdr->fw_ver, hdr->build_date); \
>> + \
>> + ret = mt7996_mcu_send_ram_firmware(dev, hdr, fw->data, \
>> + MT7996_RAM_TYPE_##_type); \
>> + if (ret) { \
>> + dev_err(dev->mt76.dev, "Failed to start %s firmware\n", #_type);\
>> + release_firmware(fw); \
>> + return ret; \
>> + } \
>> + \
>> + release_firmware(fw); \
>> + } while (0)
>> +
>> + LOAD_RAM(WM);
>> + LOAD_RAM(DSP);
>> + LOAD_RAM(WA);
>> +#undef LOAD_RAM
>
> I think it would be better to used a regular function instead of a macro, the
> code would be much easier to read.
Yeah, a function is preferred. I think Andrew Morton says: prefer C over
CPP
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
