[{"id":3320203,"web_url":"http://patchwork.ozlabs.org/comment/3320203/","msgid":"<20240531095650.GD8682@google.com>","list_archive_url":null,"date":"2024-05-31T09:56:50","subject":"Re: [DO NOT MERGE v8 23/36] mfd: sm501: Convert platform_data to OF\n property","submitter":{"id":65833,"url":"http://patchwork.ozlabs.org/api/people/65833/","name":"Lee Jones","email":"lee@kernel.org"},"content":"On Wed, 29 May 2024, Yoshinori Sato wrote:\n\n> Various parameters of SM501 can be set using platform_data,\n> so parameters cannot be passed in the DeviceTree target.\n> Expands the parameters set in platform_data so that they can be\n> specified using DeviceTree properties.\n> \n> Signed-off-by: Yoshinori Sato \n> ---\n> drivers/mfd/sm501.c | 238 ++++++++++++++++++++++++++++++++++\n> drivers/video/fbdev/sm501fb.c | 87 +++++++++++++\n> 2 files changed, 325 insertions(+)\n> \n> diff --git a/drivers/mfd/sm501.c b/drivers/mfd/sm501.c\n> index b3592982a83b..d373aded0c3b 100644\n> --- a/drivers/mfd/sm501.c\n> +++ b/drivers/mfd/sm501.c\n> @@ -20,6 +20,7 @@\n> #include \n> #include \n> #include \n> +#include \n> \n> #include \n> #include \n> @@ -82,6 +83,16 @@ struct sm501_devdata {\n> \tunsigned int\t\t\t rev;\n> };\n> \n> +struct sm501_config_props_uint {\n> +\tchar *name;\n> +\tu32 shift;\n> +};\n> +\n> +struct sm501_config_props_flag {\n> +\tchar *clr_name;\n> +\tchar *set_name;\n> +\tu32 bit;\n> +};\n> \n> #define MHZ (1000 * 1000)\n> \n> @@ -1370,6 +1381,227 @@ static int sm501_init_dev(struct sm501_devdata *sm)\n> \treturn 0;\n> }\n> \n> +#define FIELD_WIDTH 4\n> +struct dt_values {\n> +\tchar *name;\n> +\tunsigned int offset;\n> +\tunsigned int width;\n> +\tchar *val[(1 << FIELD_WIDTH) + 1];\n> +};\n> +\n> +#define fld(_name, _offset, _width, ...)\t\\\n> +\t{ \\\n> +\t\t.name = _name, \\\n> +\t\t.offset = _offset, \\\n> +\t\t.width = _width,\t\\\n> +\t\t.val = { __VA_ARGS__, NULL},\t\\\n> +\t}\n> +\n> +static const struct dt_values misc_timing[] = {\n> +\tfld(\"ex\", 28, 4,\n> +\t \"none\", \"16\", \"32\", \"48\", \"64\", \"80\", \"96\", \"112\",\n> +\t \"128\", \"144\", \"160\", \"176\", \"192\", \"208\", \"224\", \"240\"),\n> +\tfld(\"xc\", 24, 2, \"internal-pll\", \"hclk\", \"gpio30\"),\n> +\tfld(\"us\", 23, 1, \"disable\", \"enable\"),\n> +\tfld(\"ssm1\", 20, 1, \"288\", \"divider\"),\n> +\tfld(\"sm1\", 16, 4,\n> +\t \"1\", \"2\", \"4\", \"8\", \"16\", \"32\", \"64\", \"128\",\n> +\t \"3\", \"6\", \"12\", \"24\", \"48\", \"96\", \"192\", \"384\"),\n> +\tfld(\"ssm0\", 12, 1, \"288\", \"divider\"),\n> +\tfld(\"sm0\", 8, 4,\n> +\t \"1\", \"2\", \"4\", \"8\", \"16\", \"32\", \"64\", \"128\",\n> +\t \"3\", \"6\", \"12\", \"24\", \"48\", \"96\", \"192\", \"384\"),\n> +\tfld(\"deb\", 7, 1, \"input-reference\", \"output\"),\n> +\tfld(\"a\", 6, 1, \"no-acpi\", \"acpi\"),\n> +\tfld(\"divider\", 4, 2, \"336\", \"288\", \"240\", \"192\"),\n> +\tfld(\"u\", 3, 1, \"normal\", \"simulation\"),\n> +\tfld(\"delay\", 0, 3, \"none\", \"0.5\", \"1.0\", \"1.5\", \"2.0\", \"2.5\"),\n> +\t{ .name = NULL },\n> +};\n> +\n> +static const struct dt_values misc_control[] = {\n> +\tfld(\"pad\", 30, 2, \"24\", \"12\", \"8\"),\n> +\tfld(\"usbclk\", 28, 2, \"xtal\", \"96\", \"48\"),\n> +\tfld(\"ssp\", 27, 1, \"uart1\", \"ssp1\"),\n> +\tfld(\"lat\", 26, 1, \"disable\", \"enable\"),\n> +\tfld(\"fp\", 25, 1, \"18\", \"24\"),\n> +\tfld(\"freq\", 24, 1, \"24\", \"12\"),\n> +\tfld(\"refresh\", 21, 2, \"8\", \"16\", \"32\", \"64\"),\n> +\tfld(\"hold\", 18, 3, \"fifo-empty\", \"8\", \"16\", \"24\", \"32\"),\n> +\tfld(\"sh\", 17, 1, \"active-low\", \"active-high\"),\n> +\tfld(\"ii\", 16, 1, \"normal\", \"inverted\"),\n> +\tfld(\"pll\", 15, 1, \"disable\", \"enable\"),\n> +\tfld(\"gap\", 13, 2, \"0\"),\n> +\tfld(\"dac\", 12, 1, \"enable\", \"disable\"),\n> +\tfld(\"mc\", 11, 1, \"cpu\", \"8051\"),\n> +\tfld(\"bl\", 10, 8, \"1\"),\n> +\tfld(\"usb\", 9, 1, \"master\", \"slave\"),\n> +\tfld(\"vr\", 4, 1, \"0x1e00000\", \"0x3e00000\"),\n> +\t{ .name = NULL },\n> +};\n\nI've been avoiding this set for a while now!\n\nI appreciate the amount of work that you've put into this, but this is a\nbit of a disaster. It's a hell of lot of over-complex infrastructure\njust to pull out some values from DT.\n\nForgive me if I have this wrong, but it looks like you're defining\nvarious structs then populating static versions with hard-coded offsets\ninto DT arrays! Then you have a bunch of hoop-jumpy functions to\nfirstly parse the offset-structs, then conduct look-ups to pull the\nfinal value which in turn gets shifted into an encoded variable ready\nfor to write out to the registers. Bonkers.\n\nWhat does 'timing' even mean in this context? Clocks?\n\nWhat other devices require this kind of handling? Why is this device so\ndifferent from all other supported devices to date? Instead of\nattempting to shoehorn this into a 20 year old driver, why not reshape\nit to bring it into alignment with how we do things today?\n\nE.g. handle all clocking from the clock driver, all display settings\n(including timing?) from the display driver, etc.","headers":{"Return-Path":"\n ","X-Original-To":["incoming@patchwork.ozlabs.org","linux-pci@vger.kernel.org"],"Delivered-To":"patchwork-incoming@legolas.ozlabs.org","Authentication-Results":["legolas.ozlabs.org;\n\tdkim=pass (2048-bit key;\n unprotected) header.d=kernel.org header.i=@kernel.org header.a=rsa-sha256\n header.s=k20201202 header.b=hu5bMmVB;\n\tdkim-atps=neutral","legolas.ozlabs.org;\n spf=pass (sender SPF authorized) smtp.mailfrom=vger.kernel.org\n (client-ip=2604:1380:45e3:2400::1; helo=sv.mirrors.kernel.org;\n envelope-from=linux-pci+bounces-8112-incoming=patchwork.ozlabs.org@vger.kernel.org;\n receiver=patchwork.ozlabs.org)","smtp.subspace.kernel.org;\n\tdkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org\n header.b=\"hu5bMmVB\"","smtp.subspace.kernel.org;\n arc=none smtp.client-ip=10.30.226.201"],"Received":["from sv.mirrors.kernel.org (sv.mirrors.kernel.org\n [IPv6:2604:1380:45e3:2400::1])\n\t(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)\n\t key-exchange X25519 server-signature ECDSA (secp384r1))\n\t(No client certificate requested)\n\tby legolas.ozlabs.org (Postfix) with ESMTPS id 4VrJSS5HgMz20QB\n\tfor ; Fri, 31 May 2024 19:57:12 +1000 (AEST)","from smtp.subspace.kernel.org (wormhole.subspace.kernel.org\n [52.25.139.140])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))\n\t(No client certificate requested)\n\tby sv.mirrors.kernel.org (Postfix) with ESMTPS id D3DF32827F5\n\tfor ; Fri, 31 May 2024 09:57:10 +0000 (UTC)","from localhost.localdomain (localhost.localdomain [127.0.0.1])\n\tby smtp.subspace.kernel.org (Postfix) with ESMTP id A8E551420A8;\n\tFri, 31 May 2024 09:57:05 +0000 (UTC)","from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org\n [10.30.226.201])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))\n\t(No client certificate requested)\n\tby smtp.subspace.kernel.org (Postfix) with ESMTPS id 5D3CF1CD35;\n\tFri, 31 May 2024 09:57:05 +0000 (UTC)","by smtp.kernel.org (Postfix) with ESMTPSA id 65A56C116B1;\n\tFri, 31 May 2024 09:56:53 +0000 (UTC)"],"ARC-Seal":"i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116;\n\tt=1717149425; cv=none;\n b=G3XdTga2krT/U3hOn7davQ/wxbcVaccYs1RIR6zOWy/ymL1O18Uxk2TnmpfKewfBaXHgCVy6YGkBnowmQg99yLuNZdQvRU0Fph2w6xYo/pYgjNns2IQUEB3btyerL+v/4VCirNJQiQ6aZcQu34S2YtjEacbmSYGTZXKlYIyLGww=","ARC-Message-Signature":"i=1; a=rsa-sha256; d=subspace.kernel.org;\n\ts=arc-20240116; t=1717149425; c=relaxed/simple;\n\tbh=pG1FuJNpg4iD5VvuUxyaJG1TPUBzdsFzeembK8i33cE=;\n\th=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version:\n\t Content-Type:Content-Disposition:In-Reply-To;\n b=QSooPByyUkRjkWzBjLh9BoUUR0xWLdDPQmL7VUmXupIGXM6npxUaaZ4ppyNso7km0sJopNLEs4TVdEg5IgahXYmSWpUKq5934p0e6xDaSUltaW92PinNGwqv020vsZa3iIBGASytf3rySiuJNqEWwmCnncAqwLZ6wB96oRCi8RE=","ARC-Authentication-Results":"i=1; smtp.subspace.kernel.org;\n dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org\n header.b=hu5bMmVB; arc=none smtp.client-ip=10.30.226.201","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org;\n\ts=k20201202; t=1717149424;\n\tbh=pG1FuJNpg4iD5VvuUxyaJG1TPUBzdsFzeembK8i33cE=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=hu5bMmVBc986akPDt9Ge9tIAY28LTFO3zwiOt/Joe+hTEGWcqkK1uspvfXfUgXXxe\n\t 37xIAg+mkLYdlb3Hs9WdiXpCF1VeQrUhl78CihP1szlXYNSBa15MdkJ/xVV25GxxSe\n\t GoFHLTigYu0YGr12LUzpQPJDP6vTnVpL49ILbXi3zxAMPLAoxFNkYdK1EVTmNJFn7f\n\t CA1vM3tFKh3BQ8+a2Mrj8F6+o+ltODCoNMa/d4HHrTNiLP+FE/f+tft+dne1biNKt8\n\t EM2SBRlBln82aT4Aeemi64mL4//CHv+Iem7UQVhtCKN9l9VGh3B5qbmCfUa1q0Ty4h\n\t r2ZCGXufem1Qw==","Date":"Fri, 31 May 2024 10:56:50 +0100","From":"Lee Jones ","To":"Yoshinori Sato ","Cc":"linux-sh@vger.kernel.org, Damien Le Moal ,\n Niklas Cassel , Rob Herring ,\n Krzysztof Kozlowski , Conor Dooley ,\n Geert Uytterhoeven ,\n Michael Turquette , Stephen Boyd ,\n David Airlie , Daniel Vetter ,\n Maarten Lankhorst ,\n Maxime Ripard , Thomas Zimmermann ,\n Thomas Gleixner , Bjorn Helgaas ,\n Lorenzo Pieralisi , Krzysztof =?utf-8?q?Wilczy=C5=84?=\n\t=?utf-8?q?ski?= ,\n Greg Kroah-Hartman ,\n Jiri Slaby , Magnus Damm ,\n Daniel Lezcano , Rich Felker ,\n John Paul Adrian Glaubitz ,\n Helge Deller , Heiko Stuebner ,\n Neil Armstrong ,\n Chris Morgan , Sebastian Reichel ,\n Linus Walleij , Arnd Bergmann ,\n Masahiro Yamada , Baoquan He ,\n Andrew Morton ,\n Guenter Roeck , Kefeng Wang ,\n Stephen Rothwell ,\n Azeem Shaikh , Guo Ren ,\n Max Filippov , Jernej Skrabec ,\n Herve Codina ,\n Andy Shevchenko ,\n Anup Patel , Jacky Huang ,\n Hugo Villeneuve , Jonathan Corbet ,\n Wolfram Sang , Uwe =?iso-8859-1?q?Kleine-?=\n\t=?iso-8859-1?q?K=F6nig?= ,\n Christophe JAILLET ,\n Sam Ravnborg ,\n Javier Martinez Canillas ,\n Sergey Shtylyov ,\n Laurent Pinchart ,\n linux-ide@vger.kernel.org, devicetree@vger.kernel.org,\n linux-kernel@vger.kernel.org, linux-renesas-soc@vger.kernel.org,\n linux-clk@vger.kernel.org, dri-devel@lists.freedesktop.org,\n linux-pci@vger.kernel.org, linux-serial@vger.kernel.org,\n linux-fbdev@vger.kernel.org","Subject":"Re: [DO NOT MERGE v8 23/36] mfd: sm501: Convert platform_data to OF\n property","Message-ID":"<20240531095650.GD8682@google.com>","References":"\n ","Precedence":"bulk","X-Mailing-List":"linux-pci@vger.kernel.org","List-Id":"","List-Subscribe":"","List-Unsubscribe":"","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"\n "}}]