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=-8.8 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,MENTIONS_GIT_HOSTING, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=unavailable 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 8ABBBC433E0 for ; Fri, 26 Mar 2021 12:29:02 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 455D8619F3 for ; Fri, 26 Mar 2021 12:29:02 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230054AbhCZM2g (ORCPT ); Fri, 26 Mar 2021 08:28:36 -0400 Received: from mga05.intel.com ([192.55.52.43]:49167 "EHLO mga05.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229794AbhCZM2I (ORCPT ); Fri, 26 Mar 2021 08:28:08 -0400 IronPort-SDR: 6n9a/SYibYIIK9vWNu43kNPPIwkh6ZtsD4Yu6qffdh/yPJ4cRjA+WLAlFzhNvjn73wpV87Bd+x pzCu0Uxz0K1g== X-IronPort-AV: E=McAfee;i="6000,8403,9934"; a="276277920" X-IronPort-AV: E=Sophos;i="5.81,280,1610438400"; d="scan'208";a="276277920" Received: from orsmga001.jf.intel.com ([10.7.209.18]) by fmsmga105.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 26 Mar 2021 05:28:07 -0700 IronPort-SDR: gZvRIMX2zE0NlnCe5Q+/ojNZsssjV9b4Ea8lq/R3SeJO3MghC4vYrh3IS5tM6qdLetuRE7c3zv rGNfp/yAExoA== X-IronPort-AV: E=Sophos;i="5.81,280,1610438400"; d="scan'208";a="453496980" Received: from smile.fi.intel.com (HELO smile) ([10.237.68.40]) by orsmga001-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 26 Mar 2021 05:28:05 -0700 Received: from andy by smile with local (Exim 4.94) (envelope-from ) id 1lPlZK-00GONp-Um; Fri, 26 Mar 2021 14:28:02 +0200 Date: Fri, 26 Mar 2021 14:28:02 +0200 From: Andy Shevchenko To: Daniel Gomez Cc: Jarkko Nikula , Mika Westerberg , linux-i2c@vger.kernel.org, Linux Kernel Mailing List Subject: Re: [PATCH] i2c: designware: Add base addr info Message-ID: References: <20210325151248.1066643-1-daniel@qtec.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Mar 26, 2021 at 11:35:08AM +0100, Daniel Gomez wrote: > 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. You are welcome! ... > > > 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? It's a good question. On one hand we know that resource_size_t is a simple redefinition of phys_addr_t, but it might be changed in the future. OTOH, struct resource has types of resource_size_t. In any case it's a type that is platform dependent (like long, size_t). Hence, the special specifier is needed. > 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. Not all examples in the kernel are good examples (many of them is a cargo-cult and / or outdated). Take them with grain of salt. But common rule is to check the log of the interesting subsystem (`git log -- drivers//`) in order to find the most recent drivers or modules added. There you very likely will find more or less modern standards and APIs you might reuse in your code. > 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. There is a document that describes all possible extensions we have for %p. You might be curious to read more there. ... > > > + 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 TTY is different. TTY often related to a console and it's very important to know some information as soon as possible (don't forget also hardware debuggers, e.g. Lauterbach, which able to show kernel message ring buffer). As you may know console is the first common target during new platform bring-up. -- With Best Regards, Andy Shevchenko