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=-0.8 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS autolearn=no 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 6EDCBC2BA19 for ; Wed, 22 Apr 2020 01:40:24 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id EF29920724 for ; Wed, 22 Apr 2020 01:40:23 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="JOgfozM+"; dkim=fail reason="signature verification failed" (1024-bit key) header.d=chromium.org header.i=@chromium.org header.b="EQfvQRYk" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org EF29920724 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=chromium.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:To:Subject:Message-ID:Date:From: In-Reply-To:References:MIME-Version:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=mK/2QHtUlaodAh6G0vlHbVR3IqtzvO1WhFYres8Lk+8=; b=JOgfozM+bkCjrC uk7XAlwF2xEb+DiVpr3OV0I9wIM8Yvm1opKm8CS87jgeoTc+BN3FhdibmEF5mmlC7B1XbeD0TtBny Ma687rwPJTZKbqSelMfECHau/jWMqbiyxaG+5KHyakUAtI1BVj3TL7AUD3y7EllmoE7W8e3MHnogj aHjXtO82XJq3Z+blHaNRjjuQp97FqriAv+IKXJ8nrWb3hLB1hakDM+BfLvfISvq3OmxA2COS4BJR1 vbyVtU6FUWjTWiTg6mbARMTSrwauhX8PWyx2XfJAU89TbmDlPyl9dYoreR+b0sRF432nqTB2SoJgO W3u/bBK35SJJGFWh+6Ng==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1jR4N9-0004bB-U2; Wed, 22 Apr 2020 01:40:19 +0000 Received: from mail-pl1-x644.google.com ([2607:f8b0:4864:20::644]) by bombadil.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1jR4N6-0004Zc-Ii for linux-arm-kernel@lists.infradead.org; Wed, 22 Apr 2020 01:40:17 +0000 Received: by mail-pl1-x644.google.com with SMTP id h11so251347plr.11 for ; Tue, 21 Apr 2020 18:40:15 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=fCEr6pXIIH8a6wvkqNzUF108BpUBYKo9yQQ8CIUa6jU=; b=EQfvQRYkh1XrsrAz+l9+x0k+vOSp/UM4fgbCnYQ64D1Dq8B1FWcLG+ca/ZEvrLDjeK Lh2aJ9ttT5kKR1+EZokWVRzp6P4jAQiExqYM9Uq7+iSd8vkAer6L1ObRJBmwUbr+Saq+ DKZprigLH+OXKegSJ60ezTDhpCv/UN11QS9qQ= 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=fCEr6pXIIH8a6wvkqNzUF108BpUBYKo9yQQ8CIUa6jU=; b=gkJRHGJg7/OYVtEbFJ2YZ1g8sGlqqgfw1kqnnQ71AsQGrYp+S0G6CBEnB44iaFcbh/ YJ7muMULgexPDcXrPkcLcbQfC7+bDb2ZBs2SCQH5ELuQ6XGB8EJLbns5MRVyTzFQAE5F Y4+lc5ne/VFgT18f23QzAoTZfLng+39QHqJI36vSG60IjvGPuliSw4j/FxurMA2oUlgp B7Eu94Pg1uSqxvgdEysgxGn5fODmx+gNGdJMbNseOP9++2Hm6v+c+n3DiJDBeuPi+tO6 Ln0PXEDgFK2QB/HOsDlhA0h4UeakswTCujm8x2RCHqqBrZ3bYuDHVCeUwPTIUcMGzcoC UvBA== X-Gm-Message-State: AGi0PuYLX6YMFtyK6mmfrVw2qOmgDhuQGqPh1SN96DHSsCn4ffRhP9Yb nYCuJ7Ar7E04Z47cHjyLZ9z/g7p230o= X-Google-Smtp-Source: APiQypJ1CcXSfy4Q6BB1nmYmv1WbnefQ+cG9sX6maRb1ZEaA4gd1WcLENbEX7+mDq95BtAc6Z1s61A== X-Received: by 2002:a17:902:b708:: with SMTP id d8mr4696857pls.69.1587519614723; Tue, 21 Apr 2020 18:40:14 -0700 (PDT) Received: from mail-pl1-f174.google.com (mail-pl1-f174.google.com. [209.85.214.174]) by smtp.gmail.com with ESMTPSA id o185sm3665005pfg.197.2020.04.21.18.40.13 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 21 Apr 2020 18:40:14 -0700 (PDT) Received: by mail-pl1-f174.google.com with SMTP id v2so255519plp.9 for ; Tue, 21 Apr 2020 18:40:13 -0700 (PDT) X-Received: by 2002:a67:907:: with SMTP id 7mr17409548vsj.42.1587519611840; Tue, 21 Apr 2020 18:40:11 -0700 (PDT) MIME-Version: 1.0 References: <20200421110520.197930-1-evanbenn@chromium.org> <20200421210403.v2.2.Ia92bb4d4ce84bcefeba1d00aaa1c1e919b6164ef@changeid> In-Reply-To: From: Evan Benn Date: Wed, 22 Apr 2020 11:39:44 +1000 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH v2 2/2] watchdog: Add new arm_smc_wdt watchdog driver To: Julius Werner X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20200421_184016_635648_6F5A40D9 X-CRM114-Status: GOOD ( 23.27 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Geert Uytterhoeven , Catalin Marinas , Bjorn Andersson , Leonard Crestez , Will Deacon , Xingyu Chen , Rob Herring , Anson Huang , Mauro Carvalho Chehab , Marcin Juszkiewicz , Valentin Schneider , Guenter Roeck , LINUX-WATCHDOG , "moderated list:ARM/Mediatek SoC support" , Matthias Brugger , Wim Van Sebroeck , "moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE" , Greg Kroah-Hartman , LKML , Li Yang , Olof Johansson , Shawn Guo , "David S. Miller" Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Wed, Apr 22, 2020 at 6:31 AM Julius Werner wrote: > > > +static int smcwd_call(unsigned long smc_func_id, enum smcwd_call call, > > + unsigned long arg, struct arm_smccc_res *res) > > I think you should just take a struct watchdog_device* here and do the > drvdata unpacking inside the function. That makes sense, I avoided it because smcwd_call's are made during 'probe', ~while we are 'constructing' the wdd. But this is C, so I think I have permission to do this! > > +static int smcwd_probe(struct platform_device *pdev) > > +{ > > + struct watchdog_device *wdd; > > + int err; > > + struct arm_smccc_res res; > > + u32 *smc_func_id; > > + > > + smc_func_id = > > + devm_kzalloc(&pdev->dev, sizeof(*smc_func_id), GFP_KERNEL); > > + if (!smc_func_id) > > + return -ENOMEM; > > nit: Could save the allocation by just casting the value itself to a > pointer? Or is that considered too hacky? I am not yet used to what hacks are allowed in the kernel. Where I learned C that would not be allowed. I assumed the kernel allocator has fast paths for tiny sizes though. > > +static const struct of_device_id smcwd_dt_ids[] = { > > + { .compatible = "mediatek,mt8173-smc-wdt" }, > > + {} > > +}; > > +MODULE_DEVICE_TABLE(of, smcwd_dt_ids); > > So I'm a bit confused about this... I thought the plan was to either > use arm,smc-id and then there'll be no reason to put platform-specific > quirks into the driver, so we can just use a generic "arm,smc-wdt" > compatible string on all platforms; or we put individual compatible > strings for each platform and use them to hardcode platform-specific > differences (like the SMC ID) in the driver. But now you're kinda > doing both by making the driver code platform-independent but still > using a platform-specific compatible string, that doesn't seem to fit > together. (If the driver can be platform independent, I think it's > nicer to have a generic compatible string so that future platforms > which support the same interface don't have to land code changes in > order to just use the driver.) Yes I think you are correct. I got some reviews about the compatible name, but I think I misinterpreted those, and arm,smc-wdt would work. I did wonder if Xingyu from amlogic needed to modify the driver more, EG with different SMCWD_enum values for the amlogic chip, he could then just add an amlogic compatible and keep our devices running with the other compatible. Although of course it would be nicer if the amlogic firmware could copy the values here. Thanks Evan _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel