From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-10.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, MENTIONS_GIT_HOSTING,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id E128AC433DB for ; Fri, 26 Mar 2021 10:36:06 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 9C02561A4A for ; Fri, 26 Mar 2021 10:36:06 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229794AbhCZKfi (ORCPT ); Fri, 26 Mar 2021 06:35:38 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:57486 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229671AbhCZKfV (ORCPT ); Fri, 26 Mar 2021 06:35:21 -0400 Received: from mail-lf1-x130.google.com (mail-lf1-x130.google.com [IPv6:2a00:1450:4864:20::130]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 2B494C0613AA for ; Fri, 26 Mar 2021 03:35:21 -0700 (PDT) Received: by mail-lf1-x130.google.com with SMTP id a198so6945463lfd.7 for ; Fri, 26 Mar 2021 03:35:21 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=qtec.com; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=P4iiVAZ5wP8F1gMdbkc2yTj4+SB47g4kbs/gxBR2nNI=; b=cXPt7TC6AP+FPpXRAylbkVHBX8VrtuqmQCLCjjoUppbn1AGIdfzL9Blx2a8T9k/0MH AzaXq95xgxSO63ZF0fqeHislkgrfFBU0FaZrIeC3Bbb1UvBbs/oVWdzaGsTgB46CY+6A RwrooQnWch01lgWQaP6ml52uOTvF90XVcA6nXXMu7onkJYjJ6NLpWUrvep6seY17D9vy kFo8XXzSc4zK1vMnvZvjA6ofeU013BHrAAMHXUc3QKmEmwQbZaC0wivt8PWXREU3aV/u CuYRzAUsFbKT99+W0+U3eKNDQIAVAJ5IvFFU+f0siHX5Qd1Xfxt8TEc0yRhjlo4ckzqy Iv9A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=P4iiVAZ5wP8F1gMdbkc2yTj4+SB47g4kbs/gxBR2nNI=; b=PaByNLxvnDimVYim0w4Vx45kzKha/KqqVn+JVpOzzDVd53J3W+ei+8wK4RU0KT3tA/ mzomssdco9z9cpS5awdsvVj1KAp4B6pNLe7UzfaaDAqyERJLkMIdbEh+Vx06M1QzbQ7H 1o542VflcfFDqrBxnI0bzP++M0OVAnxqhAvDcTvIqrd2QO3mes8nfaosOJntjeIW9jy+ 3Wc7e68sjKloFdclyZDTOKB0VkkqV7OOC1bV+Mj85mEBYIlpMyG6kiRfVYmqfdv+4IVl KbXzcjYFYcUu7Ntt7YSbxLYVuuDOCpmm5huLT9JSNJwdHrd5iu/JZ+Wu70Hwu14dYPt7 LV8g== X-Gm-Message-State: AOAM531RrkAEhXbmhpdE1kBM8ijXPtH8YHb5Fys4Po3+fEXAwgfz+ddf 1RYFYHqe8hSIQVaneJpR5tRgcbQ5m5a5k+m1MpZafn/7MfX8wA== X-Google-Smtp-Source: ABdhPJxes2GHS2I259EZkK8DNmpEKttkMA6NAtpRq5kcLuizqeY3sCSZdNZGpJ74F2KhU/5URWiN7Czzl9p4sZyx810= X-Received: by 2002:a19:ec13:: with SMTP id b19mr7962856lfa.238.1616754919658; Fri, 26 Mar 2021 03:35:19 -0700 (PDT) MIME-Version: 1.0 References: <20210325151248.1066643-1-daniel@qtec.com> In-Reply-To: From: Daniel Gomez Date: Fri, 26 Mar 2021 11:35:08 +0100 Message-ID: Subject: Re: [PATCH] i2c: designware: Add base addr info To: Andy Shevchenko Cc: Jarkko Nikula , Mika Westerberg , linux-i2c@vger.kernel.org, Linux Kernel Mailing List Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Andy, On Thu, 25 Mar 2021 at 16:43, Andy Shevchenko wrote: > > On Thu, Mar 25, 2021 at 04:12:48PM +0100, Daniel Gomez wrote: > > Add i2c hw base address in the adapter name and when the device is > > probed. > > Why? > We have /proc/iomem for that. The initial reason was because I wasn't aware of /proc/iomem therefore I didn't have a way to match the physical address to the i2c adapter. So, thanks for pointing that out as now I'm able to match the physical address listed in iomem with the sysfs i2c bus. > > Sorry, I don't see value of this change. > Some comments below just to let you know about style:ish issues. Thanks for the comments. Although there are no reasons to apply this patch I have some doubts perhaps they will help me next time. > > ... > > > snprintf(adap->name, sizeof(adap->name), > > - "Synopsys DesignWare I2C adapter"); > > + "Synopsys DesignWare I2C adapter at 0x%llx", dev->base_addr); > > It actually should be resource_size_t and corresponding specifier, i.e. %pa to > print it. Moreover, we have %pR (and %pr) specifiers for struct resource. I understand this but I had some doubts when I declared the variable. I took this as reference: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/i2c/busses/i2c-tegra.c?h=v5.12-rc4#n268 Should it be then defined as resource_size_t instead? Out of the i2c subsystem, I also found several examples. For example this: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/iio/adc/at91-sama5d2_adc.c?h=v5.12-rc4#n364 But I understand this could be out of the scope. Some others, even assign the the start to the dma_addr_t which could vary depending on CONFIG_ARCH_DMA_ADDR_T_64BIT but it seems equivalent to the phys_addr_t definition. > > ... > > > + dev_info(&pdev->dev, "%s\n", adap->name); > > Unneeded noise. Also this might be out of the scope again but I added because in tty they were printing that information: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/tty/serial/serial_core.c?h=v5.12-rc4#n2336 Anyway, thanks again Andy for the review. Really appreciate it! :) > > -- > With Best Regards, > Andy Shevchenko > >