From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wr1-f50.google.com (mail-wr1-f50.google.com [209.85.221.50]) (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 52FF78806 for ; Tue, 24 Jan 2023 14:42:27 +0000 (UTC) Received: by mail-wr1-f50.google.com with SMTP id bk16so14081348wrb.11 for ; Tue, 24 Jan 2023 06:42:27 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=ULYuWXV/gOEakuXjYyzzPaFAF/ufwa1YfwMolryeGUM=; b=cgw3mJmyiwBWjJtV9xeWFnFIArTiv+hIHox3q8JHJd1tEiIc95OhiLNb+Esl7v2uk/ l9j6UQA8fmquOTUmBmWw46iI+3ZFYjM2z0RFxDxwYqDwIS1IGjUUpEvCn8YsHdIYBfR4 GBcLi0y+5KVuNclKgoHqAK8kw4w7BUjWIHPa/iaNCtCZ9UhDIwHGSmSiGPxJ62ycV8Ly MCJu5HZI4GzabKTMv+82IT+Eh7AzcsrMrffut+SJPkINivgb9UKhY6y0k/G+JJx0zIk5 qACMp3TpagVHMu0zMZghJnAqfl8Pfbi7aaY6qUBj1E/r4XofX+oEk09Yy+UswNOQvfSs gqTQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=ULYuWXV/gOEakuXjYyzzPaFAF/ufwa1YfwMolryeGUM=; b=ZZfB2oy8EP91rXZtjlPOo/9HO3MpV9Go3DiXfLNuS96Ybe1fc93LQ77bPh9vrQi6aB nDE/SGUkPQIOOsevioP0ZClG1on4KUUa/vIXwZGu61ViH376HdX5d6AYAjgHTvjlDyK5 PCB3oG0ZqfnDIJAB0DCyDOBxK6PlDM7yS6gZLSOmehuiQND/W5R8bUkOZ4MsxOvREk5W ISlVrnPM63s4+GB50WPL/VqIQBFHSr5kWTIpIHGfYZD7ldQFP7UotVy5nF+c+mvCHeSA pCRGqgmzSGmYfeP76sNpJyw+AhIPgM611z4YT3pXCcxgZh1K6bIfZD15acgbqhc1QJEe SpdQ== X-Gm-Message-State: AFqh2krEquLss5ULbTtYjtb8el1au6WUZ7pGPuOXgleQMqaMpwCQxnvF SNcFGhKhaSdS5xJaksrJr78= X-Google-Smtp-Source: AMrXdXskPF1ZD0ZmfencZ5tEi8M4YEL2aTheDKQMrqEBrm7RB+dZMcEceObH2/QvJlVxFhODeOG0wg== X-Received: by 2002:a05:6000:549:b0:2be:184a:5d5c with SMTP id b9-20020a056000054900b002be184a5d5cmr21672099wrf.59.1674571345340; Tue, 24 Jan 2023 06:42:25 -0800 (PST) Received: from localhost ([102.36.222.112]) by smtp.gmail.com with ESMTPSA id m14-20020adffa0e000000b00287da7ee033sm2098028wrr.46.2023.01.24.06.42.24 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 24 Jan 2023 06:42:24 -0800 (PST) Date: Tue, 24 Jan 2023 17:42:21 +0300 From: Dan Carpenter To: Brent Pappas Cc: sakari.ailus@linux.intel.com, bingbu.cao@intel.com, tian.shu.qiu@intel.com, mchehab@kernel.org, gregkh@linuxfoundation.org, linux-media@vger.kernel.org, linux-staging@lists.linux.dev, linux-kernel@vger.kernel.org Subject: Re: [PATCH] mdeia: ipu3: ipu33-mmu: Replace macro IPU3_ADDR2PTE() with a function Message-ID: References: <20230124135554.13787-1-bpappas@pappasbrent.com> Precedence: bulk X-Mailing-List: linux-staging@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20230124135554.13787-1-bpappas@pappasbrent.com> On Tue, Jan 24, 2023 at 08:55:54AM -0500, Brent Pappas wrote: > Replace the macro IPU3_ADDR2PTE() with a static function to match > Linux coding style standards. When you say "Linux coding style standards" what exactly does that mean? I've just re-read the Documentation/process/coding-style.rst section on "Macros, Enums and RTL" and I don't see an issue with the macro. This code is in the middle of a big section full of macros. Why did you pick this particular macro? Now it doesn't mirror the IPU3_PTE2ADDR() so this patch hurts readability. > > Signed-off-by: Brent Pappas > --- > drivers/staging/media/ipu3/ipu3-mmu.c | 18 +++++++++++------- > 1 file changed, 11 insertions(+), 7 deletions(-) > > diff --git a/drivers/staging/media/ipu3/ipu3-mmu.c b/drivers/staging/media/ipu3/ipu3-mmu.c > index cb9bf5fb29a5..d2d603c32773 100644 > --- a/drivers/staging/media/ipu3/ipu3-mmu.c > +++ b/drivers/staging/media/ipu3/ipu3-mmu.c > @@ -25,7 +25,11 @@ > #define IPU3_PT_SIZE (IPU3_PT_PTES << 2) > #define IPU3_PT_ORDER (IPU3_PT_SIZE >> PAGE_SHIFT) > > -#define IPU3_ADDR2PTE(addr) ((addr) >> IPU3_PAGE_SHIFT) > +static u32 ipu3_addr2pte(phys_addr_t addr) > +{ > + return addr >> IPU3_PAGE_SHIFT; > +} To me the original macro is fine. The inline would also be fine if it were done consistently. But I guess I just don't see a lot of value in changing the existing code. If you were taking ownership of this driver in a more meaningful way then I would defer to your taste... But I just don't see a lot of value in the patch. regards, dan carpenter