Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S934058AbbEMKln (ORCPT ); Wed, 13 May 2015 06:41:43 -0400 Received: from mail-wi0-f172.google.com ([209.85.212.172]:35954 "EHLO mail-wi0-f172.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933478AbbEMKlj (ORCPT ); Wed, 13 May 2015 06:41:39 -0400 MIME-Version: 1.0 In-Reply-To: <20150513101625.GY10961@lukather> References: <1431462804-30467-1-git-send-email-maxime.ripard@free-electrons.com> <20150513093441.80359.qmail@dec59.ruk.cuni.cz> <20150513101625.GY10961@lukather> From: Michal Suchanek Date: Wed, 13 May 2015 12:40:58 +0200 Message-ID: Subject: Re: [PATCH] spi: Add option to bind spidev to all chipselects To: Maxime Ripard Cc: Mark Brown , Greg Kroah-Hartman , Linux Kernel Mailing List , Hans de Goede , linux-spi , Martin Sperl Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2405 Lines: 66 On 13 May 2015 at 12:16, Maxime Ripard wrote: > Hi, > > On Wed, May 13, 2015 at 09:34:41AM -0000, Michal Suchanek wrote: >> Bypass the check if CS is in use for spidev devices if CONFIG_SPIDEV_SHADOW is >> set. Rename spidev devices to avoid sysfs conflict. >> >> This allows dynamically loading SPI device overlays or communicating >> with SPI devices configured by a kernel driver from userspace. >> >> Signed-off-by: Michal Suchanek > > Output from checkpatch: > total: 2 errors, 4 warnings, 4 checks, 157 lines checked > > ... > > I told you a few times already to run checkpatch before sending your > patches, apparently you make a point at ignoring me. Fine. That's a good idea to run, yes. Sorry about that. I also discovered an additional issue with unused variable when the config option is set. > > That being said, I'm not sure this is the right approach, or at least, > it doesn't solve anything. If SPIDEV_SHADOW is not set, you will still > have the same issue with addition of new devices on previously unused > chip selects, and where we have an spidev device now. > > What I think we should do is, when a new device is created, we just > lookup the modalias of the spi_device associated to it. > > If that modalias is "spidev", then unregister the spidev device, > register the new device, you're done. If not, return an error. Yes, that's what I intend to look into eventually. However, this patch is still useful and allows both accessing unused bus with spidev and dynamically loading overlays that would use the bus. > > On the SPIDEV_SHADOW stuff itself, I'm not sure this is such a good > idea. There's a good chance it will break the driver by doing stuff > behind its back, possibly in a way that will harm the whole kernel, > and it's something we usually try to avoid. What is the possibility to harm the whole kernel? If the kernel crashes because the device misses a message this is somewhat worrying. You could see it as a developer option similar to SCSI error injection and others that can introduce states that would normally occur only rarely. Thanks Michal -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/