All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jarkko Nikula <jarkko.nikula-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
To: Marcel Holtmann <marcel-kz+m5ild9QBg9hUCZPvPmw@public.gmane.org>
Cc: linux-bluetooth
	<linux-bluetooth-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	linux-acpi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	"Gustavo F. Padovan"
	<gustavo-THi1TnShQwVAfugRpC6u6w@public.gmane.org>,
	Johan Hedberg
	<johan.hedberg-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
	Loic Poulain
	<loic.poulain-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Subject: Re: [PATCH] Bluetooth: hci_intel: Cleanup the device probe code
Date: Thu, 01 Oct 2015 09:26:56 +0300	[thread overview]
Message-ID: <560CD230.9050008@linux.intel.com> (raw)
In-Reply-To: <9BB7E6C8-A45E-4D88-8B01-D1660370B7FF-kz+m5ild9QBg9hUCZPvPmw@public.gmane.org>

On 09/30/2015 05:52 PM, Marcel Holtmann wrote:
> Hi Jarkko,
>
>> There is some unneeded code in "hci_intel" probing. First
>> acpi_match_device() call is needless as driver/platform/acpi core code has
>> already done the matching before calling the probe and the driver does not
>> use the returned pointer to matching _HID other than checking is it NULL.
>>
>> Then tree wide grep for "hci_intel" doesn't reveal that there is any code
>> registering this platform device so it looks this device is always backed
>> with ACPI companion so also ACPI_HANDLE() test can be removed.
>
> eventually there will be DT platform device. Does that make a difference or is it still valid to remove this probe handling.
>
Also with DT matching the acpi_match_device() is needless. Adding DT 
support here means just adding pointer to OF match table like below and 
core will call the probe if there is a match to "hci_intel" platform 
device, ACPI _HID or OF node.

static struct platform_driver intel_driver = {
	.driver = {
		.name = "hci_intel",
		.acpi_match_table = ACPI_PTR(intel_acpi_match),
		.of_match_table = of_match_ptr(intel_of_match),
	},
};

If there is need to differentiate between these in probe the driver 
could do for instance

if (has_acpi_companion(&pdev->dev)) {
	/* ACPI enumerated */
} else if (pdev->dev.of_node) {
	/* DT enumerated */
} else {
	/* platform device */
}

-- 
Jarkko

WARNING: multiple messages have this Message-ID (diff)
From: Jarkko Nikula <jarkko.nikula@linux.intel.com>
To: Marcel Holtmann <marcel@holtmann.org>
Cc: linux-bluetooth <linux-bluetooth@vger.kernel.org>,
	linux-acpi@vger.kernel.org,
	"Gustavo F. Padovan" <gustavo@padovan.org>,
	Johan Hedberg <johan.hedberg@gmail.com>,
	Loic Poulain <loic.poulain@intel.com>
Subject: Re: [PATCH] Bluetooth: hci_intel: Cleanup the device probe code
Date: Thu, 01 Oct 2015 09:26:56 +0300	[thread overview]
Message-ID: <560CD230.9050008@linux.intel.com> (raw)
In-Reply-To: <9BB7E6C8-A45E-4D88-8B01-D1660370B7FF@holtmann.org>

On 09/30/2015 05:52 PM, Marcel Holtmann wrote:
> Hi Jarkko,
>
>> There is some unneeded code in "hci_intel" probing. First
>> acpi_match_device() call is needless as driver/platform/acpi core code has
>> already done the matching before calling the probe and the driver does not
>> use the returned pointer to matching _HID other than checking is it NULL.
>>
>> Then tree wide grep for "hci_intel" doesn't reveal that there is any code
>> registering this platform device so it looks this device is always backed
>> with ACPI companion so also ACPI_HANDLE() test can be removed.
>
> eventually there will be DT platform device. Does that make a difference or is it still valid to remove this probe handling.
>
Also with DT matching the acpi_match_device() is needless. Adding DT 
support here means just adding pointer to OF match table like below and 
core will call the probe if there is a match to "hci_intel" platform 
device, ACPI _HID or OF node.

static struct platform_driver intel_driver = {
	.driver = {
		.name = "hci_intel",
		.acpi_match_table = ACPI_PTR(intel_acpi_match),
		.of_match_table = of_match_ptr(intel_of_match),
	},
};

If there is need to differentiate between these in probe the driver 
could do for instance

if (has_acpi_companion(&pdev->dev)) {
	/* ACPI enumerated */
} else if (pdev->dev.of_node) {
	/* DT enumerated */
} else {
	/* platform device */
}

-- 
Jarkko

  parent reply	other threads:[~2015-10-01  6:26 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-09-30 13:28 [PATCH] Bluetooth: hci_intel: Cleanup the device probe code Jarkko Nikula
     [not found] ` <1443619732-19401-1-git-send-email-jarkko.nikula-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
2015-09-30 14:52   ` Marcel Holtmann
2015-09-30 14:52     ` Marcel Holtmann
     [not found]     ` <9BB7E6C8-A45E-4D88-8B01-D1660370B7FF-kz+m5ild9QBg9hUCZPvPmw@public.gmane.org>
2015-10-01  6:26       ` Jarkko Nikula [this message]
2015-10-01  6:26         ` Jarkko Nikula
2015-10-01  6:57 ` Marcel Holtmann

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=560CD230.9050008@linux.intel.com \
    --to=jarkko.nikula-vuqaysv1563yd54fqh9/ca@public.gmane.org \
    --cc=gustavo-THi1TnShQwVAfugRpC6u6w@public.gmane.org \
    --cc=johan.hedberg-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \
    --cc=linux-acpi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-bluetooth-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=loic.poulain-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org \
    --cc=marcel-kz+m5ild9QBg9hUCZPvPmw@public.gmane.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.