From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wr1-f44.google.com (mail-wr1-f44.google.com [209.85.221.44]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 4601D3C0A for ; Mon, 18 Jul 2022 19:33:12 +0000 (UTC) Received: by mail-wr1-f44.google.com with SMTP id d16so18492205wrv.10 for ; Mon, 18 Jul 2022 12:33:12 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=message-id:date:mime-version:user-agent:subject:content-language:to :references:from:in-reply-to:content-transfer-encoding; bh=xwOi0/YqmOj47DIS9XkftfFeH7Ad13wjAtHDSHbI450=; b=dGi9GSHC3uV2Tx+fDM28pxHAquxspdxN8voOAbb77fxkDwlidDStGMPx0u1hlsg0Tc C+P8FNd53X+vyHsguz+nyUbW18/k6VGIdk3iNfN0FatsqpowDxnL+j1fGlibN7HP08pb Nq4E1Pj9i3facXWRddVbV48qTk05INDrTnH9CWqUSzq+6gHvmerw3cDnOOHP8ZBySB+r zyhEwqp0FIs2SMZ3sXpeJTXM8pDsZcIngeJ3uyb4XETFL7749EocVjR8JdjrTSJgMpPe 44SljfFn1L5ZFAfGPtSzw9MwV1EmBIa1U55qsvFPvibrwdS2u4b1cfgV6l307WeN0gd8 Ckzg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:date:mime-version:user-agent:subject :content-language:to:references:from:in-reply-to :content-transfer-encoding; bh=xwOi0/YqmOj47DIS9XkftfFeH7Ad13wjAtHDSHbI450=; b=6xfdNij7a8dXeF6eAZ2zgAFoGM/XsrKii54UT6ZAWVWQ/uOoe7vG1ieXItvE7dTXTf sgSEJfYvkDsf3mqcLmOZspMe1xyvmu7ESiLHc8sO9PxEWus4Cgpt25TzVfjYvnGTMVAp EZ1bgaKHiw5L1iicGOUiW8a0+KYc9tSuXb3/a4GbTOwmBsA47QlyHtQyZ5/WSr6Jah9E DYM8FsQz/Z7D2mOwWf9bjAXGytEOWiVg7LlxmTGjgWpX3egdL6bKSOr1Vd3xYybzkUJN jmuL3iLIO4q5bned9UTBimCqYg//BfomMyLVLi4FtEJpW1gYBw5dlvFACAJzElfn19aY o/Fw== X-Gm-Message-State: AJIora/gVNYQP0Omgdm9g+iA70gdms7VQSXGIQ2RVb59ad0M+pWF3I+v o6fMjZ9eLGYyLOKKvntyhufYnxeiddQ= X-Google-Smtp-Source: AGRyM1tZcdrTnOWuKMDCpj3dhGwlJ+rKD67mfQ2bDQGJl9jWR4hOcIzV8+LPffjMUcNiEGlPaGCjaQ== X-Received: by 2002:a05:6000:1ac8:b0:21d:b7d9:3c03 with SMTP id i8-20020a0560001ac800b0021db7d93c03mr23155250wry.149.1658172790485; Mon, 18 Jul 2022 12:33:10 -0700 (PDT) Received: from ?IPV6:2003:c7:8f2e:6999:b072:faf1:1e0f:8765? (p200300c78f2e6999b072faf11e0f8765.dip0.t-ipconnect.de. [2003:c7:8f2e:6999:b072:faf1:1e0f:8765]) by smtp.gmail.com with ESMTPSA id m185-20020a1c26c2000000b003a302fb9df7sm16061235wmm.21.2022.07.18.12.33.09 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 18 Jul 2022 12:33:10 -0700 (PDT) Message-ID: Date: Mon, 18 Jul 2022 21:33:08 +0200 Precedence: bulk X-Mailing-List: linux-staging@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.11.0 Subject: Re: [PATCH 1/7] staging: vt6655: Rename dwData to reg_value in four macros Content-Language: en-US To: Joe Perches , Forest Bond , Greg Kroah-Hartman , linux-staging@lists.linux.dev, linux-kernel@vger.kernel.org References: From: Philipp Hortmann In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit On 7/18/22 08:07, Joe Perches wrote: > Please remember that checkpatch is a stupid little scripted tool > and the actual goal is to have readable code. Understood. > > Look a bit beyond the code and see if and how you could make the > code better. > > All of these macros have the same form and logic. > That is the reason why I put them all together in one static function: static void vt6655_mac_dma_ctl(void __iomem *iobase, u8 reg_index) { unsigned long reg_value; reg_value = ioread32(iobase + reg_index); if (reg_value & DMACTL_RUN) iowrite32(DMACTL_WAKE, iobase + reg_index); else iowrite32(DMACTL_RUN, iobase + reg_index); } > Perhaps it'd be better to use another indirect macro and define > all of these with that new macro. > > Something like: > > #define mac_v(iobase, reg) \ > do { \ > void __iomem *addr = (iobase) + (reg); \ > iowrite32(ioread32(addr) & DMACTL_RUN ? DMACTL_WAKE : DMACTL_RUN,\ > addr); \ > } while (0) > > #define MACvReceive0(iobase) mac_v(iobase, MAC_REG_RXDMACTL0) > #define MACvReceive1(iobase) mac_v(iobase, MAC_REG_RXDMACTL1) > #define MACvTransmit0(iobase) mac_v(iobase, MAC_REG_TXDMACTL0) > #define MACvTransmitAC0(iobase) mac_v(iobase, MAC_REG_AC0DMACTL) That is an interesting solution. But for me this code is not as good readable as my proposal. Reason is that I struggle with the function in function with condition broken into two lines.