From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.1 (2015-04-28) on archive.lwn.net X-Spam-Level: X-Spam-Status: No, score=-4.7 required=5.0 tests=DKIM_SIGNED, HEADER_FROM_DIFFERENT_DOMAINS,RCVD_IN_DNSWL_HI,T_DKIM_INVALID, T_RP_MATCHES_RCVD autolearn=unavailable autolearn_force=no version=3.4.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by archive.lwn.net (Postfix) with ESMTP id 1928D7DE7A for ; Sat, 24 Mar 2018 21:54:34 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752095AbeCXVyb (ORCPT ); Sat, 24 Mar 2018 17:54:31 -0400 Received: from smtp.codeaurora.org ([198.145.29.96]:56594 "EHLO smtp.codeaurora.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751972AbeCXVy3 (ORCPT ); Sat, 24 Mar 2018 17:54:29 -0400 Received: by smtp.codeaurora.org (Postfix, from userid 1000) id 2898260271; Sat, 24 Mar 2018 21:54:29 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=codeaurora.org; s=default; t=1521928469; bh=S08FGV/ggNvSM1eejziJOk9RdA0gquhWMTrDH3ALg3Q=; h=Subject:To:Cc:References:From:Date:In-Reply-To:From; b=G1togWemptocL1Cj8m3Q+hSoMSjcZyKwzfpOe9VaMRSbyP1qG2Hr7Eq75x5mOyOI+ I5MDUCeZqlzeQZ4xZveqy44BI8jDRQ4PCOnzttlrsw/LfMlZ/+RVb+weSEsUoFs4h2 JKKBCR1KVufsRf7yaBTPt1UFFLjqruleh+9EPfK8= Received: from [192.168.1.6] (c-67-166-9-45.hsd1.co.comcast.net [67.166.9.45]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) (Authenticated sender: sdharia@smtp.codeaurora.org) by smtp.codeaurora.org (Postfix) with ESMTPSA id C4F5E60271; Sat, 24 Mar 2018 21:54:26 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=codeaurora.org; s=default; t=1521928467; bh=S08FGV/ggNvSM1eejziJOk9RdA0gquhWMTrDH3ALg3Q=; h=Subject:To:Cc:References:From:Date:In-Reply-To:From; b=GYPw4O/TMrsdrj9SWz2A4ygB2YrBwVvvXDDRfge4TlpuZvi8l2HTReeXGkn/Pkv5k Y/WQKpkxbtLvZzHcr7YQoQ/NsdQkRYi8XV5SeuPAoC+/HKsDbQweRBa+uf9DCLSuoR 5AeEA3M8cpdc8uKK+/DsgIBRy9kOvVwj+DZv0BGs= DMARC-Filter: OpenDMARC Filter v1.3.2 smtp.codeaurora.org C4F5E60271 Authentication-Results: pdx-caf-mail.web.codeaurora.org; dmarc=none (p=none dis=none) header.from=codeaurora.org Authentication-Results: pdx-caf-mail.web.codeaurora.org; spf=none smtp.mailfrom=sdharia@codeaurora.org Subject: Re: [PATCH v5 3/5] i2c: i2c-qcom-geni: Add bus driver for the Qualcomm GENI I2C controller To: Doug Anderson , Karthikeyan Ramasubramanian Cc: Jonathan Corbet , Andy Gross , David Brown , Rob Herring , Mark Rutland , Wolfram Sang , linux-doc@vger.kernel.org, linux-arm-msm@vger.kernel.org, devicetree@vger.kernel.org, linux-i2c@vger.kernel.org, evgreen@chromium.org, acourbot@chromium.org, swboyd@chromium.org, Girish Mahadevan References: <1521836461-6515-1-git-send-email-kramasub@codeaurora.org> <1521836461-6515-4-git-send-email-kramasub@codeaurora.org> From: Sagar Dharia Message-ID: <2feb9931-2aff-9562-b20a-0216d413ad0e@codeaurora.org> Date: Sat, 24 Mar 2018 15:54:25 -0600 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.6.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit Sender: linux-doc-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-doc@vger.kernel.org Hi Doug On 3/23/2018 5:34 PM, Doug Anderson wrote: > Hi, > > On Fri, Mar 23, 2018 at 1:20 PM, Karthikeyan Ramasubramanian > wrote: >> This bus driver supports the GENI based i2c hardware controller in the >> Qualcomm SOCs. The Qualcomm Generic Interface (GENI) is a programmable >> module supporting a wide range of serial interfaces including I2C. The >> driver supports FIFO mode and DMA mode of transfer and switches modes >> dynamically depending on the size of the transfer. >> >> Signed-off-by: Karthikeyan Ramasubramanian >> Signed-off-by: Sagar Dharia >> Signed-off-by: Girish Mahadevan >> --- >> drivers/i2c/busses/Kconfig | 13 + >> drivers/i2c/busses/Makefile | 1 + >> drivers/i2c/busses/i2c-qcom-geni.c | 650 +++++++++++++++++++++++++++++++++++++ >> 3 files changed, 664 insertions(+) > > [...] > >> +/* >> + * Hardware uses the underlying formula to calculate time periods of >> + * SCL clock cycle. Firmware uses some additional cycles excluded from the >> + * below formula and it is confirmed that the time periods are within >> + * specification limits. > > I was hoping for more than just "oh, and there's a fudge factor", but > I guess this is the best I'm going to get? > According to our HW/FW team: "We use over sampling in our FW and we use 1-2 NOPE extra in some cases. this is why we can’t give a exact formula." > >> +static int geni_i2c_probe(struct platform_device *pdev) >> +{ >> + struct geni_i2c_dev *gi2c; >> + struct resource *res; >> + u32 proto, tx_depth; >> + int ret; >> + >> + gi2c = devm_kzalloc(&pdev->dev, sizeof(*gi2c), GFP_KERNEL); >> + if (!gi2c) >> + return -ENOMEM; >> + >> + gi2c->se.dev = &pdev->dev; >> + gi2c->se.wrapper = dev_get_drvdata(pdev->dev.parent); >> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); >> + gi2c->se.base = devm_ioremap_resource(&pdev->dev, res); >> + if (IS_ERR(gi2c->se.base)) >> + return PTR_ERR(gi2c->se.base); >> + >> + gi2c->se.clk = devm_clk_get(&pdev->dev, "se"); >> + if (IS_ERR(gi2c->se.clk)) { >> + ret = PTR_ERR(gi2c->se.clk); >> + dev_err(&pdev->dev, "Err getting SE Core clk %d\n", ret); >> + return ret; >> + } >> + >> + ret = device_property_read_u32(&pdev->dev, "clock-frequency", >> + &gi2c->clk_freq_out); >> + if (ret) { >> + /* Clock frequency not specified, so default to 100kHz. */ >> + dev_info(&pdev->dev, >> + "Bus frequency not specified, default to 100kHz.\n"); > > If you happen to spin again, can you remove the comment since it's > obvious from the string in the print? It looks a lot like this code: > > /* Print hello, world */ > printf("hello, world\n"); > > > In any case, that's a pretty minor nit, so I'll add: > > Reviewed-by: Douglas Anderson > > ...assuming that the bindings and "geni" code get Acked / landed > somewhere. Ideally let's not land this before the geni code lands > since if the geni API changes for some reason it'll cause us grief. > Sure, thanks a lot for reviewing the patches! -Sagar > > -Doug > -- Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html