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 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 91C99E6FE36 for ; Fri, 22 Sep 2023 14:19:09 +0000 (UTC) Received: by smtp.kernel.org (Postfix) id 60538C433C7; Fri, 22 Sep 2023 14:19:09 +0000 (UTC) Received: from mgamail.intel.com (mgamail.intel.com [192.55.52.151]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.kernel.org (Postfix) with ESMTPS id EF85DC433C8; Fri, 22 Sep 2023 14:19:07 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 smtp.kernel.org EF85DC433C8 Authentication-Results: smtp.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: smtp.kernel.org; spf=fail smtp.mailfrom=kernel.org X-IronPort-AV: E=McAfee;i="6600,9927,10841"; a="361089089" X-IronPort-AV: E=Sophos;i="6.03,167,1694761200"; d="scan'208";a="361089089" Received: from orsmga005.jf.intel.com ([10.7.209.41]) by fmsmga107.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 22 Sep 2023 07:19:01 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10841"; a="921176897" X-IronPort-AV: E=Sophos;i="6.03,167,1694761200"; d="scan'208";a="921176897" Received: from smile.fi.intel.com ([10.237.72.54]) by orsmga005.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 22 Sep 2023 07:18:59 -0700 Received: from andy by smile.fi.intel.com with local (Exim 4.97-RC0) (envelope-from ) id 1qjgzg-0000000HCoY-1iXu; Fri, 22 Sep 2023 17:18:56 +0300 Date: Fri, 22 Sep 2023 17:18:56 +0300 From: Andy Shevchenko To: Marek =?iso-8859-1?Q?Beh=FAn?= List-Id: Cc: Gregory CLEMENT , Arnd Bergmann , soc@kernel.org, arm@kernel.org, Linus Walleij , Bartosz Golaszewski , linux-gpio@vger.kernel.org Subject: Re: [PATCH v2 3/7] platform: cznic: turris-omnia-mcu: Add support for MCU connected GPIOs Message-ID: References: <20230919103815.16818-1-kabel@kernel.org> <20230919103815.16818-4-kabel@kernel.org> <20230920190818.50b2018b@dellmb> <20230921222453.4b3bdb4c@thinkpad> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20230921222453.4b3bdb4c@thinkpad> Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo On Thu, Sep 21, 2023 at 10:25:01PM +0200, Marek Behún wrote: > On Thu, 21 Sep 2023 12:55:08 +0300 > Andy Shevchenko wrote: ... > > > > > + rising = reply[0] | (reply[2] << 8) | (reply[4] << 16) | > > > > > + (reply[6] << 24); > > > > > + falling = reply[1] | (reply[3] << 8) | (reply[5] << 16) | > > > > > + (reply[7] << 24); > > > > > > > > With a help of two masks, you can access to the both edges as to > > > > 64-bit value and simplify the code. > > > > > > Huh? As in > > > rising = reply & 0x00ff00ff00ff00ff; > > > falling = reply & 0xff00ff00ff00ff00; > > > ? > > > But then I can't or the rising bit with the corresponding falling bit > > > to get pending... > > > Or I guess i can with: > > > pending = rising & (pending >> 8); > > > > > > Am I understanding you correctly? > > > > > > But then I would need to store the mask in driver data as a 64-bit > > > value with half the data not used. Also the CPU is 32-bit. > > > > If you use proper bitmaps, perhaps this will be easier. You can use one for > > each and merge them whenever you want (with bitmap_or() call) or split (with > > bitmap_and() respectively): > > > > bitmap_or(full, raising, failing); // merge > > bitmap_and(raising, full, rasing_mask); // split > > Hmm. But then what? I or the result and use it as pending interrupt > bitmap, to be iterated over. The indexes of the bits correspond to the > constants in the MCU API. > > So after your suggestion I have rising and falling containgin > rising = 00rr00rr00rr00rr; /* r means rising bits */ > falling = 00ff00ff00ff00ff; /* f means falling bits */ > pending = rising | falling; > which means: > pending = pp00pp00pp00pp; /* p means pending bits */ > But these bit positions do not correspond to the interrupt number > anymore. > > I still think the de-interleaving of the buffer from > rr ff rr ff rr ff rr ff > into two words: > rising = rrrrrrrr; > falling = ffffffff; > is simpler... There are two sides of this: OS and hardware. See Xilinx GPIO driver how it's made there. But before going that way, check on https://lore.kernel.org/all/ZOMmuZuhdjA6mdIG@smile.fi.intel.com/ That APIs you would need I am pretty sure. ... > > > > > + if (!(mcu->features & FEAT_NEW_INT_API)) > > > > > + > > > > > cancel_delayed_work_sync(&mcu->button_release_emul_work); + > > > > > + mutex_destroy(&mcu->lock); > > > > > > > > Wrong order? > > > > > > No, the mutex may be used in the work. Can't destroy it first. Or am I > > > misunderstanding something? > > > > I mean you are using a lot of devm(), can mutex be used in IRQ or whatever > > that can be triggered after this call? > > OK, I think I need to free the irq before canceling the work. Thank you! Can you rather switch everything to be devm managed? -- With Best Regards, Andy Shevchenko