* Re: [PATCH v5 3/4] drivers/vfio: EEH support for VFIO PCI device
From: Gavin Shan @ 2014-05-22 8:11 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: aik, Alexander Graf, kvm-ppc, Gavin Shan, alex.williamson,
qiudayu, linuxppc-dev
In-Reply-To: <1400709391.29150.24.camel@pasglop>
On Thu, May 22, 2014 at 07:56:31AM +1000, Benjamin Herrenschmidt wrote:
>On Wed, 2014-05-21 at 15:07 +0200, Alexander Graf wrote:
>
>> > +#ifdef CONFIG_VFIO_PCI_EEH
>> > +int eeh_vfio_open(struct pci_dev *pdev)
>>
>> Why vfio? Also that config option will not be set if vfio is compiled as
>> a module.
>>
>> > +{
>> > + struct eeh_dev *edev;
>> > +
>> > + /* No PCI device ? */
>> > + if (!pdev)
>> > + return -ENODEV;
>> > +
>> > + /* No EEH device ? */
>> > + edev = pci_dev_to_eeh_dev(pdev);
>> > + if (!edev || !edev->pe)
>> > + return -ENODEV;
>> > +
>> > + eeh_dev_set_passed(edev, true);
>> > + eeh_pe_set_passed(edev->pe, true);
>> > +
>> > + return 0;
>> > +}
>> > +EXPORT_SYMBOL_GPL(eeh_vfio_open);
>
>Additionally, shouldn't we have some locking here ? (and in release too)
>
>I don't like relying on the caller locking (if it does it at all).
>
Ok. I'll add one mutex for open() and release() in next revision.
Thanks for the comment.
>> > + /* Device existing ? */
>> > + ret = eeh_vfio_check_dev(pdev, &edev, &pe);
>> > + if (ret) {
>> > + pr_debug("%s: Cannot find device %s\n",
>> > + __func__, pdev ? pci_name(pdev) : "NULL");
>> > + *retval = -7;
>>
>> What are these? Please use proper kernel internal return values for
>> errors. I don't want to see anything even remotely tied to RTAS in any
>> of these patches.
>
>Hint: -ENODEV
>
In next revision, Those exported functions will have return value as:
>= 0: carrried information to caller.
< 0: error number.
Thanks,
Gavin
^ permalink raw reply
* Re: [PATCH] powerpc: fix typo 'CONFIG_PMAC'
From: Paul Bolle @ 2014-05-22 7:10 UTC (permalink / raw)
To: Andreas Schwab; +Cc: Paul Mackerras, linuxppc-dev, linux-kernel
In-Reply-To: <874n0j3yjf.fsf@igel.home>
Andreas,
On Wed, 2014-05-21 at 18:08 +0200, Andreas Schwab wrote:
> Paul Bolle <pebolle@tiscali.nl> writes:
>
> > Commit b0d278b7d3ae ("powerpc/perf_event: Reduce latency of calling
> > perf_event_do_pending") added a check for CONFIG_PMAC were a check for
> > CONFIG_PPC_PMAC was clearly intended.
> >
> > Fixes: b0d278b7d3ae ("powerpc/perf_event: Reduce latency of calling perf_event_do_pending")
> > Signed-off-by: Paul Bolle <pebolle@tiscali.nl>
> > ---
> > Untested. Needs testing on 32 bit powermac, I guess.
> >
> > This typo was introduced in v2.6.36. No one noticed because very few
> > people still use 32 bit powermacs?
>
> How does that bug manifest itself?
Do you want to know how to test this patch on a 32 bit powermac? Ie, see
if it has any effect, and whether that effect improves things or make
things worse.
I have no clue how to do that, sorry. Perhaps someone else has.
Paul Bolle
^ permalink raw reply
* Re: [PATCH V2 2/3] powerpc, ptrace: Enable support for transactional memory register sets
From: Michael Ellerman @ 2014-05-22 5:30 UTC (permalink / raw)
To: Pedro Alves
Cc: mikey, avagin, Roland McGrath, oleg, linux-kernel, linuxppc-dev,
Anshuman Khandual
In-Reply-To: <5372547D.8040209@redhat.com>
On Tue, 2014-05-13 at 18:21 +0100, Pedro Alves wrote:
> I wonder whether people are getting Roland's address from?
>
> It's frequent that ptrace related patches end up CCed to
> roland@redhat.com, but, he's not been at Red Hat for a few years
> now. Roland, do you still want to be CCed on ptrace-related
> issues? If so, there's probably a script somewhere in the
> kernel that needs updating. If not, well, it'd be good
> if it were updated anyway. :-)
In MAINTAINERS I see:
PTRACE SUPPORT
M: Roland McGrath <roland@redhat.com>
M: Oleg Nesterov <oleg@redhat.com>
S: Maintained
cheers
^ permalink raw reply
* Re: [PATCH RFC v13 0/5] MPC512x DMA slave s/g support, OF DMA lookup
From: Vinod Koul @ 2014-05-22 5:10 UTC (permalink / raw)
To: Alexander Popov
Cc: devicetree, Lars-Peter Clausen, Arnd Bergmann, Gerhard Sittig,
Andy Shevchenko, dmaengine, Dan Williams, Anatolij Gustschin,
linuxppc-dev
In-Reply-To: <1400163335-29853-1-git-send-email-a13xp0p0v88@gmail.com>
On Thu, May 15, 2014 at 06:15:30PM +0400, Alexander Popov wrote:
> 2013/7/14 Gerhard Sittig <gsi@denx.de>:
> > this series
> > - introduces slave s/g support (that's support for DMA transfers which
> > involve peripherals in contrast to mem-to-mem transfers)
> > - adds device tree based lookup support for DMA channels
> > - combines floating patches and related feedback which already covered
> > several aspects of what the suggested LPB driver needs, to demonstrate
> > how integration might be done
>
> ...
>
> Changes in v12:
> A new patch (part 2/7) is added to this series.
> Part 6/7:
> - change the description of 'compatible' property according part 2/7;
> - improve the document according Gerhard's feedback;
>
> Parts 1/7, 2/7 and 4/7 have been applied by Vinod Koul and
> are excluded from v13.
>
> Changes in v13:
> A new patch (part 1/5) is added to this series.
> Part 2/5:
> - fix style issue;
> - improve comments;
You need to cc DT- list for 3 to 5 patches. Need ack before we can apply.
I suspect 4/5 should come first out of these 3.
Also in future pls use right subsystem name "dmaengine". Also why are these
tagged as RFC still?
--
~Vinod
^ permalink raw reply
* Re: [PATCH RFC v13 2/5] dma: mpc512x: add support for peripheral transfers
From: Vinod Koul @ 2014-05-22 5:08 UTC (permalink / raw)
To: Alexander Popov
Cc: Lars-Peter Clausen, Arnd Bergmann, Gerhard Sittig,
Andy Shevchenko, dmaengine, Dan Williams, Anatolij Gustschin,
linuxppc-dev
In-Reply-To: <1400163335-29853-3-git-send-email-a13xp0p0v88@gmail.com>
On Thu, May 15, 2014 at 06:15:32PM +0400, Alexander Popov wrote:
> Introduce support for slave s/g transfer preparation and the associated
> device control callback in the MPC512x DMA controller driver, which adds
> support for data transfers between memory and peripheral I/O to the
> previously supported mem-to-mem transfers.
Applied, after fixing sybstem name to "dmaengine"
--
~Vinod
^ permalink raw reply
* Re: [PATCH RFC v13 1/5] dmaengine: fix comment typo
From: Vinod Koul @ 2014-05-22 5:07 UTC (permalink / raw)
To: Alexander Popov
Cc: Lars-Peter Clausen, Arnd Bergmann, Gerhard Sittig,
Andy Shevchenko, dmaengine, Dan Williams, Anatolij Gustschin,
linuxppc-dev
In-Reply-To: <1400163335-29853-2-git-send-email-a13xp0p0v88@gmail.com>
On Thu, May 15, 2014 at 06:15:31PM +0400, Alexander Popov wrote:
> Fix comment typo.
Applied, thanks
--
~Vinod
^ permalink raw reply
* Re: [PATCH V2 2/3] powerpc, ptrace: Enable support for transactional memory register sets
From: Anshuman Khandual @ 2014-05-22 5:08 UTC (permalink / raw)
To: Pedro Alves; +Cc: mikey, avagin, oleg, linux-kernel, michael, linuxppc-dev
In-Reply-To: <537B2F5E.4040102@redhat.com>
On 05/20/2014 04:03 PM, Pedro Alves wrote:
> On 05/20/2014 09:14 AM, Anshuman Khandual wrote:
>> On 05/19/2014 08:13 PM, Pedro Alves wrote:
>>> On 05/19/2014 12:46 PM, Anshuman Khandual wrote:
>>>
>>>>>> I couldn't actually find any arch that currently returns -ENODEV in
>>>>>> the "active" hook. I see that binfmt_elf.c doesn't handle
>>>>>> regset->active() returning < 0. Guess that may be why. Looks like
>>>>>> something that could be cleaned up, to me.
>>>>>>
>>>> Also it does not consider the return value of regset->active(t->task, regset)
>>>> (whose objective is to figure out whether we need to request regset->n number
>>>> of elements or less than that) in the subsequent call to regset->get function.
>>>
>>> Indeed.
>>>
>>> TBC, do you plan on fixing this? Otherwise ...
>>
>> Sure, thinking something like this as mentioned below. But still not sure how to use
>> the return type of -ENODEV from the function regset->active(). Right now if any
>> regset does have the active hook and it returns anything but positive value, it will
>> be ignored and the control moves to the next regset in view. This prevents the thread
>> core note type being written to the core dump.
>
> Looks to me that that's exactly what should happen for -ENODEV too. The regset
> should be ignored. If regset->active() returns -ENODEV, then the machine
> doesn't have the registers at all, so what makes sense to me is to not write the
> corresponding core note in the dump. IOW, on such a machine, the kernel
> generates a core exactly like if the support for these registers that don't
> make sense for this machine wasn't compiled in at all. And generates a core
> exactly like an older kernel that didn't know about that regset
> (which is fine for that same machine) yet.
>
All of this happen right now even without specifically checking for the return type
of -ENODEV and just checking for a positive value. I guess thats the reason they had
omitted -ENODEV in the first place.
>>
>> diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
>> index aa3cb62..80672fb 100644
>> --- a/fs/binfmt_elf.c
>> +++ b/fs/binfmt_elf.c
>> @@ -1553,7 +1553,15 @@ static int fill_thread_core_info(struct elf_thread_core_info *t,
>> if (regset->core_note_type && regset->get &&
>> (!regset->active || regset->active(t->task, regset))) {
>> int ret;
>
> So, here, this ?
>
> (!regset->active || regset->active(t->task, regset) > 0)) {
>
>
>> - size_t size = regset->n * regset->size;
>> + size_t size;
>> +
>> + /* Request only the active elements in the regset */
>> + if (!regset->active)
>> + size = regset->n * regset->size;
>> + else
>> + size = regset->active(t->task, regset)
>> + * regset->size;
>> +
>
>
> I wonder if it wouldn't be cleaner to add a function like:
>
> int
> regset_active (tast *task, regseg *regset)
> {
> if (!regset->active)
> return regset->n * regset->size;
> else
> return regset->active(task, regset);
> }
>
> And then use it like
>
> if (regset->core_note_type && regset->get) {
> int size = regset_active (t->task, regset);
>
> if (size > 0) {
> ...
> }
>
Yeah this makes sense.
> Though at this point, we don't actually make use of
> the distinction between -ENODEV vs 0. Guess that's what
> we should be thinking about. Seems like there some details that
> need to be sorted out, and some verification that consumers aren't
> broken by outputting smaller notes -- e.g., ia64 makes me
> wonder that.
I agree.
>
> Maybe we should leave this for another day, and have tm_spr_active
> return 0 instead of -ENODEV when the machine doesn't have the hardware,
> or not install that hook at all. Seems like the effect will be the same,
> as the note isn't output if ->get fails.
Agree. Active hooks which return 0 in case of -ENODEV sounds good to me and shall
incorporate this in the next version.
>
>> void *data = kmalloc(size, GFP_KERNEL);
>> if (unlikely(!data))
>> return 0;
>>
>>>
>>>> Now coming to the installation of the .active hooks part for all the new regsets, it
>>>> should be pretty straight forward as well. Though its optional and used for elf_core_dump
>>>> purpose only, its worth adding them here. Example of an active function should be something
>>>> like this. The function is inexpensive as required.
>>>>
>>>> +static int tm_spr_active(struct task_struct *target,
>>>> + const struct user_regset *regset)
>>>> +{
>>>> + if (!cpu_has_feature(CPU_FTR_TM))
>>>> + return -ENODEV;
>>>
>>> ... unfortunately this will do the wrong thing.
>>
>> I am not sure whether I understand this correctly. Are you saying that its wrong to return
>> -ENODEV in this case as above ?
>
> No, sorry for not being clear. The (...)'s were connected:
>
> "do you plan on fixing this? Otherwise ... ... unfortunately
> this will do the wrong thing."
>
:)
^ permalink raw reply
* Re: [PATCH v5 4/4] powerpc/eeh: Avoid event on passed PE
From: Gavin Shan @ 2014-05-22 0:01 UTC (permalink / raw)
To: Alexander Graf
Cc: aik, Gavin Shan, kvm-ppc, alex.williamson, qiudayu, linuxppc-dev
In-Reply-To: <537CA667.2070906@suse.de>
On Wed, May 21, 2014 at 03:13:11PM +0200, Alexander Graf wrote:
>
>On 21.05.14 07:03, Gavin Shan wrote:
>>If we detects frozen state on PE that has been passed to guest, we
>>needn't handle it. Instead, we rely on the guest to detect and recover
>>it. The patch avoid EEH event on the frozen passed PE so that the guest
>>can have chance to handle that.
>>
>>Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
>>---
>> arch/powerpc/kernel/eeh.c | 8 ++++++++
>> arch/powerpc/platforms/powernv/eeh-ioda.c | 3 ++-
>> 2 files changed, 10 insertions(+), 1 deletion(-)
>>
>>diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
>>index 2aaf90e..25fd12d 100644
>>--- a/arch/powerpc/kernel/eeh.c
>>+++ b/arch/powerpc/kernel/eeh.c
>>@@ -400,6 +400,14 @@ int eeh_dev_check_failure(struct eeh_dev *edev)
>> if (ret > 0)
>> return ret;
>>+ /*
>>+ * If the PE has been passed to guest, we won't check the
>>+ * state. Instead, let the guest handle it if the PE has
>
>What guest? The kernel doesn't care whether we use VFIO for a guest or not.
>
Ok. I'll not mention "guest" and "vfio" in next revision.
Thanks,
Gavin
>
>Alex
>
>>+ * been frozen.
>>+ */
>>+ if (eeh_pe_passed(pe))
>>+ return 0;
>>+
>> /* If we already have a pending isolation event for this
>> * slot, we know it's bad already, we don't need to check.
>> * Do this checking under a lock; as multiple PCI devices
>>diff --git a/arch/powerpc/platforms/powernv/eeh-ioda.c b/arch/powerpc/platforms/powernv/eeh-ioda.c
>>index 1b5982f..03a3ed2 100644
>>--- a/arch/powerpc/platforms/powernv/eeh-ioda.c
>>+++ b/arch/powerpc/platforms/powernv/eeh-ioda.c
>>@@ -890,7 +890,8 @@ static int ioda_eeh_next_error(struct eeh_pe **pe)
>> opal_pci_eeh_freeze_clear(phb->opal_id, frozen_pe_no,
>> OPAL_EEH_ACTION_CLEAR_FREEZE_ALL);
>> ret = EEH_NEXT_ERR_NONE;
>>- } else if ((*pe)->state & EEH_PE_ISOLATED) {
>>+ } else if ((*pe)->state & EEH_PE_ISOLATED ||
>>+ eeh_pe_passed(*pe)) {
>> ret = EEH_NEXT_ERR_NONE;
>> } else {
>> pr_err("EEH: Frozen PHB#%x-PE#%x (%s) detected\n",
>
^ permalink raw reply
* Re: [PATCH v5 3/4] drivers/vfio: EEH support for VFIO PCI device
From: Gavin Shan @ 2014-05-21 23:48 UTC (permalink / raw)
To: Alexander Graf
Cc: aik, Gavin Shan, kvm-ppc, alex.williamson, qiudayu, linuxppc-dev
In-Reply-To: <537CA50E.9090404@suse.de>
On Wed, May 21, 2014 at 03:07:26PM +0200, Alexander Graf wrote:
>
>On 21.05.14 07:03, Gavin Shan wrote:
>>The patch adds new IOCTL command VFIO_EEH_OP to VFIO PCI device
>>to support EEH functionality for PCI devices, which have been
>>passed from host to guest via VFIO.
>>
>>Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
>>---
>> Documentation/vfio.txt | 6 +-
>> arch/powerpc/include/asm/eeh.h | 10 ++
>> arch/powerpc/kernel/eeh.c | 323 +++++++++++++++++++++++++++++++++++++++++
>> drivers/vfio/pci/vfio_pci.c | 99 ++++++++++++-
>> include/uapi/linux/vfio.h | 43 ++++++
>> 5 files changed, 474 insertions(+), 7 deletions(-)
>>
>>diff --git a/Documentation/vfio.txt b/Documentation/vfio.txt
>>index b9ca023..bb17ec7 100644
>>--- a/Documentation/vfio.txt
>>+++ b/Documentation/vfio.txt
>>@@ -305,7 +305,10 @@ faster, the map/unmap handling has been implemented in real mode which provides
>> an excellent performance which has limitations such as inability to do
>> locked pages accounting in real time.
>>-So 3 additional ioctls have been added:
>>+4) PPC64 guests detect PCI errors and recover from them via EEH RTAS services,
>>+which works on the basis of additional ioctl command VFIO_EEH_OP.
>>+
>>+So 4 additional ioctls have been added:
>> VFIO_IOMMU_SPAPR_TCE_GET_INFO - returns the size and the start
>> of the DMA window on the PCI bus.
>>@@ -316,6 +319,7 @@ So 3 additional ioctls have been added:
>> VFIO_IOMMU_DISABLE - disables the container.
>>+ VFIO_EEH_OP - EEH dependent operations
>
>Please document exactly what the ioctl does. In an ideal world, a
>VFIO user will just look at the documentation and be able to write a
>program against the API with it.
>
Ok. I'll amend it. Also, I'll split it to 5 ioctl commands in next revision.
>> The code flow from the example above should be slightly changed:
>>diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h
>>index 34a2d83..93922ef 100644
>>--- a/arch/powerpc/include/asm/eeh.h
>>+++ b/arch/powerpc/include/asm/eeh.h
>>@@ -305,6 +305,16 @@ void eeh_add_device_late(struct pci_dev *);
>> void eeh_add_device_tree_late(struct pci_bus *);
>> void eeh_add_sysfs_files(struct pci_bus *);
>> void eeh_remove_device(struct pci_dev *);
>>+#ifdef CONFIG_VFIO_PCI_EEH
>>+int eeh_vfio_open(struct pci_dev *pdev);
>>+void eeh_vfio_release(struct pci_dev *pdev);
>>+int eeh_vfio_set_pe_option(struct pci_dev *pdev, int option, int *retval);
>>+int eeh_vfio_get_pe_addr(struct pci_dev *pdev, int option,
>>+ int *retval, int *info);
>>+int eeh_vfio_get_pe_state(struct pci_dev *pdev, int *retval, int *state);
>>+int eeh_vfio_reset_pe(struct pci_dev *pdev, int option, int *retval);
>>+int eeh_vfio_configure_pe(struct pci_dev *pdev, int *retval);
>>+#endif
>> /**
>> * EEH_POSSIBLE_ERROR() -- test for possible MMIO failure.
>>diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
>>index 9c6b899..2aaf90e 100644
>>--- a/arch/powerpc/kernel/eeh.c
>>+++ b/arch/powerpc/kernel/eeh.c
>>@@ -1098,6 +1098,329 @@ void eeh_remove_device(struct pci_dev *dev)
>> edev->mode &= ~EEH_DEV_SYSFS;
>> }
>>+#ifdef CONFIG_VFIO_PCI_EEH
>>+int eeh_vfio_open(struct pci_dev *pdev)
>
>Why vfio? Also that config option will not be set if vfio is compiled
>as a module.
>
The interface is expected to be used by VFIO-PCI. I'll change the function
names to following ones in next revision. No "VFIO" will be seen :-)
eeh_dev_open();
eeh_dev_release();
static eeh_dev_check();
eeh_pe_set_option();
eeh_pe_get_addr();
eeh_pe_get_state();
eeh_pe_reset();
eeh_pe_configure();
Yeah, "#ifdef CONFIG_VFIO_PCI_EEH" can be removed safely in next revision.
>>+{
>>+ struct eeh_dev *edev;
>>+
>>+ /* No PCI device ? */
>>+ if (!pdev)
>>+ return -ENODEV;
>>+
>>+ /* No EEH device ? */
>>+ edev = pci_dev_to_eeh_dev(pdev);
>>+ if (!edev || !edev->pe)
>>+ return -ENODEV;
>>+
>>+ eeh_dev_set_passed(edev, true);
>>+ eeh_pe_set_passed(edev->pe, true);
>>+
>>+ return 0;
>>+}
>>+EXPORT_SYMBOL_GPL(eeh_vfio_open);
>>+
>>+void eeh_vfio_release(struct pci_dev *pdev)
>>+{
>>+ bool release_pe = true;
>>+ struct eeh_pe *pe = NULL;
>>+ struct eeh_dev *tmp, *edev;
>>+
>>+ /* No PCI device ? */
>>+ if (!pdev)
>>+ return;
>>+
>>+ /* No EEH device ? */
>>+ edev = pci_dev_to_eeh_dev(pdev);
>>+ if (!edev || !eeh_dev_passed(edev) ||
>>+ !edev->pe || !eeh_pe_passed(pe))
>>+ return;
>>+
>>+ /* Release device */
>>+ pe = edev->pe;
>>+ eeh_dev_set_passed(edev, false);
>>+
>>+ /* Release PE */
>>+ eeh_pe_for_each_dev(pe, edev, tmp) {
>>+ if (eeh_dev_passed(edev)) {
>>+ release_pe = false;
>>+ break;
>>+ }
>>+ }
>>+
>>+ if (release_pe)
>>+ eeh_pe_set_passed(pe, false);
>>+}
>>+EXPORT_SYMBOL(eeh_vfio_release);
>>+
>>+static int eeh_vfio_check_dev(struct pci_dev *pdev,
>>+ struct eeh_dev **pedev,
>>+ struct eeh_pe **ppe)
>>+{
>>+ struct eeh_dev *edev;
>>+
>>+ /* No device ? */
>>+ if (!pdev)
>>+ return -ENODEV;
>>+
>>+ edev = pci_dev_to_eeh_dev(pdev);
>>+ if (!edev || !eeh_dev_passed(edev) ||
>>+ !edev->pe || !eeh_pe_passed(edev->pe))
>>+ return -ENODEV;
>>+
>>+ if (pedev)
>>+ *pedev = edev;
>>+ if (ppe)
>>+ *ppe = edev->pe;
>>+
>>+ return 0;
>>+}
>>+
>>+int eeh_vfio_set_pe_option(struct pci_dev *pdev, int option, int *retval)
>>+{
>>+ struct eeh_dev *edev;
>>+ struct eeh_pe *pe;
>>+ int ret = 0;
>>+
>>+ /* Device existing ? */
>>+ ret = eeh_vfio_check_dev(pdev, &edev, &pe);
>>+ if (ret) {
>>+ pr_debug("%s: Cannot find device %s\n",
>>+ __func__, pdev ? pci_name(pdev) : "NULL");
>>+ *retval = -7;
>
>What are these? Please use proper kernel internal return values for
>errors. I don't want to see anything even remotely tied to RTAS in
>any of these patches.
>
It's the return value for RTAS call "ibm,set-eeh-option". Yeah, it
makes sense to return kernel internal values for errors and will
amend in next revision.
>>+ goto out;
>>+ }
>>+
>>+ /* Invalid option ? */
>>+ if (option < EEH_OPT_DISABLE ||
>>+ option > EEH_OPT_THAW_DMA) {
>
>This is quite confusing to read because it's not obvious what is in
>between these. Just make this a switch() statement that lists the
>allowed options. Gcc will be smart enough to optimize that into a
>bounds check.
>
Ok. Will use "switch()" in next revision.
>>+ pr_debug("%s: Option %d out of range (%d, %d)\n",
>>+ __func__, option, EEH_OPT_DISABLE, EEH_OPT_THAW_DMA);
>>+ *retval = -3;
>>+ ret = -EINVAL;
>>+ goto out;
>>+ }
>>+
>>+ if (option == EEH_OPT_DISABLE ||
>>+ option == EEH_OPT_ENABLE) {
>>+ *retval = 0;
>>+ } else {
>>+ if (!eeh_ops || !eeh_ops->set_option) {
>>+ *retval = -7;
>>+ ret = -ENOENT;
>>+ goto out;
>>+ }
>>+
>>+ ret = eeh_ops->set_option(pe, option);
>>+ if (ret) {
>>+ pr_debug("%s: Failure %d from backend\n",
>>+ __func__, ret);
>>+ *retval = -1;
>>+ goto out;
>>+ }
>>+
>>+ *retval = 0;
>>+ }
>>+out:
>>+ return ret;
>>+}
>>+EXPORT_SYMBOL_GPL(eeh_vfio_set_pe_option);
>>+
>>+int eeh_vfio_get_pe_addr(struct pci_dev *pdev, int option,
>>+ int *retval, int *info)
>>+{
>>+ struct pci_bus *bus;
>>+ struct eeh_dev *edev;
>>+ struct eeh_pe *pe;
>>+ int ret = 0;
>>+
>>+ /* Device existing ? */
>>+ ret = eeh_vfio_check_dev(pdev, &edev, &pe);
>>+ if (ret) {
>>+ *retval = -3;
>>+ goto out;
>>+ }
>>+
>>+ /* Invalid option ? */
>>+ if (option != 0 && option != 1) {
>
>0? 1? What? Don't these have names? And again, please use a switch()
>for this function.
>
Will have names in next revision and use "switch()", thanks.
>>+ pr_debug("%s: option %d out of range (0, 1)\n",
>>+ __func__, option);
>>+ *retval = -3;
>>+ ret = -EINVAL;
>>+ goto out;
>>+ }
>>+
>>+ /*
>>+ * Fill result according to option. We don't differentiate
>>+ * PCI bus and device dependent PE here. So all PEs are
>>+ * built in "shared" mode. Also, the PE address has the format
>>+ * of "00BBSS00".
>>+ */
>>+ if (option == 0) {
>>+ bus = eeh_pe_bus_get(pe);
>>+ if (!bus) {
>>+ *retval = -3;
>>+ ret = -ENODEV;
>>+ goto out;
>>+ }
>>+
>>+ *retval = 0;
>>+ *info = bus->number << 16;
>
>How about positive numbers for the number and negative ones for errors?
>
We needn't carry error numbers by "info" because "retval" or "ret" already
had that information :-)
>>+ } else {
>>+ *retval = 0;
>>+ *info = 1;
>>+ }
>>+out:
>>+ return ret;
>>+}
>>+EXPORT_SYMBOL_GPL(eeh_vfio_get_pe_addr);
>>+
>>+int eeh_vfio_get_pe_state(struct pci_dev *pdev, int *retval, int *state)
>>+{
>>+ struct eeh_dev *edev;
>>+ struct eeh_pe *pe;
>>+ int result, ret = 0;
>>+
>>+ /* Device existing ? */
>>+ ret = eeh_vfio_check_dev(pdev, &edev, &pe);
>>+ if (ret) {
>>+ *retval = -3;
>>+ goto out;
>>+ }
>>+
>>+ if (!eeh_ops || !eeh_ops->get_state) {
>>+ pr_debug("%s: Unsupported request\n",
>>+ __func__);
>>+ ret = -ENOENT;
>>+ *retval = -3;
>>+ goto out;
>>+ }
>>+
>>+ result = eeh_ops->get_state(pe, NULL);
>>+ if (!(result & EEH_STATE_RESET_ACTIVE) &&
>>+ (result & EEH_STATE_DMA_ENABLED) &&
>>+ (result & EEH_STATE_MMIO_ENABLED))
>>+ *state = 0;
>>+ else if (result & EEH_STATE_RESET_ACTIVE)
>>+ *state = 1;
>>+ else if (!(result & EEH_STATE_RESET_ACTIVE) &&
>>+ !(result & EEH_STATE_DMA_ENABLED) &&
>>+ !(result & EEH_STATE_MMIO_ENABLED))
>>+ *state = 2;
>>+ else if (!(result & EEH_STATE_RESET_ACTIVE) &&
>>+ (result & EEH_STATE_DMA_ENABLED) &&
>>+ !(result & EEH_STATE_MMIO_ENABLED))
>>+ *state = 4;
>>+ else
>>+ *state = 5;
>
>What are these numbers?
>
Will have names in next revision :)
>>+
>>+ *retval = 0;
>>+out:
>>+ return ret;
>>+}
>>+EXPORT_SYMBOL_GPL(eeh_vfio_get_pe_state);
>>+
>>+int eeh_vfio_reset_pe(struct pci_dev *pdev, int option, int *retval)
>>+{
>>+ struct eeh_dev *edev;
>>+ struct eeh_pe *pe;
>>+ int ret = 0;
>>+
>>+ /* Device existing ? */
>>+ ret = eeh_vfio_check_dev(pdev, &edev, &pe);
>>+ if (ret) {
>>+ *retval = -3;
>>+ goto out;
>>+ }
>>+
>>+ /* Invalid option ? */
>>+ if (option != EEH_RESET_DEACTIVATE &&
>>+ option != EEH_RESET_HOT &&
>>+ option != EEH_RESET_FUNDAMENTAL) {
>>+ pr_debug("%s: Unsupported option %d\n",
>>+ __func__, option);
>>+ ret = -EINVAL;
>>+ *retval = -3;
>>+ goto out;
>>+ }
>>+
>>+ if (!eeh_ops || !eeh_ops->set_option || !eeh_ops->reset) {
>>+ pr_debug("%s: Unsupported request\n",
>>+ __func__);
>>+ ret = -ENOENT;
>>+ *retval = -7;
>>+ goto out;
>>+ }
>>+
>>+ ret = eeh_ops->reset(pe, option);
>>+ if (ret) {
>>+ pr_debug("%s: Failure %d from backend\n",
>>+ __func__, ret);
>>+ *retval = -1;
>>+ goto out;
>>+ }
>>+
>>+ /*
>>+ * The PE is still in frozen state and we need clear that.
>>+ * It's good to clear frozen state after deassert to avoid
>>+ * messy IO access during reset, which might cause recrusive
>
>recursive
>
Thanks.
>>+ * frozen PE.
>>+ */
>>+ if (option == EEH_RESET_DEACTIVATE) {
>>+ ret = eeh_ops->set_option(pe, EEH_OPT_THAW_MMIO);
>>+ if (ret) {
>>+ pr_debug("%s: Cannot enable IO for PHB#%d-PE#%d (%d)\n",
>>+ __func__, pe->phb->global_number, pe->addr, ret);
>>+ *retval = -1;
>>+ goto out;
>>+ }
>>+
>>+ ret = eeh_ops->set_option(pe, EEH_OPT_THAW_DMA);
>>+ if (ret) {
>>+ pr_debug("%s: Cannot enable DMA for PHB#%d-PE#%d (%d)\n",
>>+ __func__, pe->phb->global_number, pe->addr, ret);
>>+ *retval = -1;
>>+ goto out;
>>+ }
>>+
>>+ eeh_pe_state_clear(pe, EEH_PE_ISOLATED);
>>+ }
>>+
>>+ *retval = 0;
>>+out:
>>+ return ret;
>>+}
>>+EXPORT_SYMBOL_GPL(eeh_vfio_reset_pe);
>>+
>>+int eeh_vfio_configure_pe(struct pci_dev *pdev, int *retval)
>>+{
>>+ struct eeh_dev *edev;
>>+ struct eeh_pe *pe;
>>+ int ret = 0;
>>+
>>+ /* Device existing ? */
>>+ ret = eeh_vfio_check_dev(pdev, &edev, &pe);
>>+ if (ret) {
>>+ *retval = -3;
>>+ goto out;
>>+ }
>>+
>>+ /*
>>+ * The access to PCI config space on VFIO device has some
>>+ * limitations. Part of PCI config space, including BAR
>>+ * registers are not readable and writable. So the guest
>>+ * should have stale values for those registers and we have
>>+ * to restore them in host side.
>
>I don't understand this comment. When is "configure_pe" called in the
>first place? Please provide proper function descriptions for each of
>these exported functions that tell someone who may want to use them
>what they do.
>
Yeah, I'll add the description here (also in Documentation/vfio.txt).
>Also, don't mention VFIO or guests in any function inside this file.
>
Ok. I'll avoid mentioning "VFIO" and "guest".
>>+ */
>>+ eeh_pe_restore_bars(pe);
>>+ *retval = 0;
>>+
>>+out:
>>+ return ret;
>>+}
>>+EXPORT_SYMBOL_GPL(eeh_vfio_configure_pe);
>>+
>>+#endif /* CONFIG_VFIO_PCI_EEH */
>>+
>> static int proc_eeh_show(struct seq_file *m, void *v)
>> {
>> if (!eeh_enabled()) {
>>diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
>>index 7ba0424..05c3dde 100644
>>--- a/drivers/vfio/pci/vfio_pci.c
>>+++ b/drivers/vfio/pci/vfio_pci.c
>>@@ -25,6 +25,9 @@
>> #include <linux/types.h>
>> #include <linux/uaccess.h>
>> #include <linux/vfio.h>
>>+#ifdef CONFIG_VFIO_PCI_EEH
>>+#include <asm/eeh.h>
>>+#endif
>> #include "vfio_pci_private.h"
>>@@ -152,32 +155,57 @@ static void vfio_pci_disable(struct vfio_pci_device *vdev)
>> pci_restore_state(pdev);
>> }
>>+static void vfio_eeh_pci_release(struct pci_dev *pdev)
>>+{
>>+#ifdef CONFIG_VFIO_PCI_EEH
>>+ eeh_vfio_release(pdev);
>>+#endif
>>+}
>>+
>> static void vfio_pci_release(void *device_data)
>> {
>> struct vfio_pci_device *vdev = device_data;
>>- if (atomic_dec_and_test(&vdev->refcnt))
>>+ if (atomic_dec_and_test(&vdev->refcnt)) {
>>+ vfio_eeh_pci_release(vdev->pdev);
>> vfio_pci_disable(vdev);
>>+ }
>> module_put(THIS_MODULE);
>> }
>>+static int vfio_eeh_pci_open(struct pci_dev *pdev)
>>+{
>>+ int ret = 0;
>>+
>>+#ifdef CONFIG_VFIO_PCI_EEH
>>+ ret = eeh_vfio_open(pdev);
>>+#endif
>>+ return ret;
>>+}
>>+
>> static int vfio_pci_open(void *device_data)
>> {
>> struct vfio_pci_device *vdev = device_data;
>>+ int ret;
>> if (!try_module_get(THIS_MODULE))
>> return -ENODEV;
>> if (atomic_inc_return(&vdev->refcnt) == 1) {
>>- int ret = vfio_pci_enable(vdev);
>>- if (ret) {
>>- module_put(THIS_MODULE);
>>- return ret;
>>- }
>>+ ret = vfio_pci_enable(vdev);
>>+ if (ret)
>>+ goto error;
>>+
>>+ ret = vfio_eeh_pci_open(vdev->pdev);
>>+ if (ret)
>>+ goto error;
>> }
>> return 0;
>>+error:
>>+ module_put(THIS_MODULE);
>>+ return ret;
>> }
>> static int vfio_pci_get_irq_count(struct vfio_pci_device *vdev, int irq_type)
>>@@ -321,6 +349,51 @@ static int vfio_pci_for_each_slot_or_bus(struct pci_dev *pdev,
>> return walk.ret;
>> }
>>+static int vfio_eeh_pci_ioctl(struct pci_dev *pdev, struct vfio_eeh_op *info)
>
>I still don't like the idea of that multiplexing ioctl. I don't see
>any benefit in it whatsoever. Just create 5 individual ioctls with
>their own simple interfaces.
>
Ok. Will split in next revision. Thanks.
>Also, this interface has nothing to do with RTAS. So don't sneak in
>RTAS error numbers anywhere ;). It's QEMU's task to convert from
>kernel error codes to RTAS error codes.
>
Ok. Will do in next revision. Thanks for your comments and time, Alex :-)
Thanks,
Gavin
>>+{
>>+ int ret = 0;
>>+
>>+#ifdef CONFIG_VFIO_PCI_EEH
>>+ switch (info->op) {
>>+ case VFIO_EEH_OP_SET_OPTION:
>>+ ret = eeh_vfio_set_pe_option(pdev,
>>+ info->option.option,
>>+ &info->option.ret);
>>+ break;
>>+ case VFIO_EEH_OP_GET_ADDR:
>>+ ret = eeh_vfio_get_pe_addr(pdev,
>>+ info->addr.option,
>>+ &info->addr.ret,
>>+ &info->addr.info);
>>+ break;
>>+ case VFIO_EEH_OP_GET_STATE:
>>+ ret = eeh_vfio_get_pe_state(pdev,
>>+ &info->state.ret,
>>+ &info->state.reset_state);
>>+ info->state.cfg_cap = 1;
>>+ info->state.pe_unavail_info = 1000;
>>+ info->state.pe_recovery_info = 0;
>>+ break;
>>+ case VFIO_EEH_OP_PE_RESET:
>>+ ret = eeh_vfio_reset_pe(pdev,
>>+ info->reset.option,
>>+ &info->reset.ret);
>>+ break;
>>+ case VFIO_EEH_OP_PE_CONFIG:
>>+ ret = eeh_vfio_configure_pe(pdev,
>>+ &info->config.ret);
>>+ default:
>>+ ret = -EINVAL;
>>+ pr_debug("%s: Cannot handle op#%d\n",
>>+ __func__, info->op);
>>+ }
>>+#else
>>+ ret = -ENOENT;
>>+#endif
>>+
>>+ return ret;
>>+}
>>+
>> static long vfio_pci_ioctl(void *device_data,
>> unsigned int cmd, unsigned long arg)
>> {
>>@@ -682,6 +755,20 @@ hot_reset_release:
>> kfree(groups);
>> return ret;
>>+ } else if (cmd == VFIO_EEH_OP) {
>>+ struct vfio_eeh_op info;
>>+ int ret = 0;
>>+
>>+ minsz = sizeof(info);
>>+ if (copy_from_user(&info, (void __user *)arg, minsz))
>>+ return -EFAULT;
>>+ if (info.argsz < minsz)
>>+ return -EINVAL;
>>+
>>+ ret = vfio_eeh_pci_ioctl(vdev->pdev, &info);
>>+ if (copy_to_user((void __user *)arg, &info, minsz))
>>+ ret = -EFAULT;
>>+ return ret;
>> }
>> return -ENOTTY;
>>diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
>>index cb9023d..518961d 100644
>>--- a/include/uapi/linux/vfio.h
>>+++ b/include/uapi/linux/vfio.h
>>@@ -455,6 +455,49 @@ struct vfio_iommu_spapr_tce_info {
>> #define VFIO_IOMMU_SPAPR_TCE_GET_INFO _IO(VFIO_TYPE, VFIO_BASE + 12)
>>+/*
>>+ * The VFIO operation struct provides way to support EEH functionality
>>+ * for PCI device that is passed from host to guest via VFIO.
>>+ */
>>+#define VFIO_EEH_OP_SET_OPTION 0
>>+#define VFIO_EEH_OP_GET_ADDR 1
>>+#define VFIO_EEH_OP_GET_STATE 2
>>+#define VFIO_EEH_OP_PE_RESET 3
>>+#define VFIO_EEH_OP_PE_CONFIG 4
>>+
>>+struct vfio_eeh_op {
>>+ __u32 argsz;
>>+ __u32 op;
>>+
>>+ union {
>>+ struct vfio_eeh_set_option {
>>+ __u32 option;
>>+ __s32 ret;
>>+ } option;
>>+ struct vfio_eeh_pe_addr {
>>+ __u32 option;
>>+ __s32 ret;
>>+ __u32 info;
>>+ } addr;
>>+ struct vfio_eeh_pe_state {
>>+ __s32 ret;
>>+ __u32 reset_state;
>>+ __u32 cfg_cap;
>>+ __u32 pe_unavail_info;
>>+ __u32 pe_recovery_info;
>>+ } state;
>>+ struct vfio_eeh_reset {
>>+ __u32 option;
>>+ __s32 ret;
>>+ } reset;
>>+ struct vfio_eeh_config {
>>+ __s32 ret;
>>+ } config;
>>+ };
>>+};
>>+
>>+#define VFIO_EEH_OP _IO(VFIO_TYPE, VFIO_BASE + 21)
>>+
>> /* ***************************************************************** */
>> #endif /* _UAPIVFIO_H */
>
>--
>To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
>the body of a message to majordomo@vger.kernel.org
>More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* Re: [PATCH RESEND net-next 4/9] net: systemport: use the new fixed PHY helpers
From: Florian Fainelli @ 2014-05-21 22:02 UTC (permalink / raw)
To: Sergei Shtylyov
Cc: Thomas Petazzoni, open list:OPEN FIRMWARE AND..., Aida Mynzhasova,
netdev, Richard Cochran, open list, Claudiu Manoil, Vitaly Bordug,
Grant Likely, open list:LINUX FOR POWERPC..., David Miller
In-Reply-To: <537D20EA.1000305@cogentembedded.com>
2014-05-21 14:55 GMT-07:00 Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>:
> On 05/22/2014 01:38 AM, Florian Fainelli wrote:
>
>> of_phy_connect_fixed_link() is becoming obsolete, and also required
>> platform code to register the fixed PHYs at the specified addresses for
>> those to be usable. Get rid of it and use the new of_phy_is_fixed_link()
>> plus of_phy_register_fixed_link() helpers to transition over the new
>> scheme.
>
>
>> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
>> ---
>> drivers/net/ethernet/broadcom/bcmsysport.c | 17 +++++++++++++++--
>> drivers/net/ethernet/broadcom/bcmsysport.h | 1 +
>> 2 files changed, 16 insertions(+), 2 deletions(-)
>
>
>> diff --git a/drivers/net/ethernet/broadcom/bcmsysport.c
>> b/drivers/net/ethernet/broadcom/bcmsysport.c
>> index d40c5b969e9e..dc708a888f80 100644
>> --- a/drivers/net/ethernet/broadcom/bcmsysport.c
>> +++ b/drivers/net/ethernet/broadcom/bcmsysport.c
>> @@ -1327,8 +1327,8 @@ static int bcm_sysport_open(struct net_device *dev)
>> /* Read CRC forward */
>> priv->crc_fwd = !!(umac_readl(priv, UMAC_CMD) & CMD_CRC_FWD);
>>
>> - priv->phydev = of_phy_connect_fixed_link(dev,
>> bcm_sysport_adj_link,
>> -
>> priv->phy_interface);
>> + priv->phydev = of_phy_connect(dev, priv->phy_dn,
>> bcm_sysport_adj_link,
>> + 0, priv->phy_interface);
>
>
> The continuation line should start on the next character after ( on the
> above line, according to the networking coding style.
Unless I am once again not following the coding style, the patch in
patchwork has this correctly, and so does my file locally:
http://patchwork.ozlabs.org/patch/351323/
--
Florian
^ permalink raw reply
* Re: [PATCH RESEND net-next 5/9] fs_enet: use the new fixed PHY helpers
From: Sergei Shtylyov @ 2014-05-21 21:58 UTC (permalink / raw)
To: Florian Fainelli, netdev
Cc: Thomas Petazzoni, open list:OPEN FIRMWARE AND..., Aida Mynzhasova,
Richard Cochran, open list, Claudiu Manoil, Vitaly Bordug,
Grant Likely, open list:LINUX FOR POWERPC..., davem
In-Reply-To: <1400708331-18088-6-git-send-email-f.fainelli@gmail.com>
On 05/22/2014 01:38 AM, Florian Fainelli wrote:
> of_phy_connect_fixed_link() is becoming obsolete, and also required
> platform code to register the fixed PHYs at the specified addresses for
> those to be usable. Get rid of it and use the new of_phy_is_fixed_link()
> plus of_phy_register_fixed_link() helpers to transition over the new
> scheme.
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
> drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c | 16 +++++++++-------
> 1 file changed, 9 insertions(+), 7 deletions(-)
> diff --git a/drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c b/drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c
> index dc80db41d6b3..d602711e00e9 100644
> --- a/drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c
> +++ b/drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c
[...]
> @@ -1029,9 +1025,15 @@ static int fs_enet_probe(struct platform_device *ofdev)
> fpi->use_napi = 1;
> fpi->napi_weight = 17;
> fpi->phy_node = of_parse_phandle(ofdev->dev.of_node, "phy-handle", 0);
> - if ((!fpi->phy_node) && (!of_get_property(ofdev->dev.of_node, "fixed-link",
> - NULL)))
> - goto out_free_fpi;
> + if (!fpi->phy_node) {
> + if (of_phy_is_fixed_link(ofdev->dev.of_node)) {
These two *if* statements could be collapsed, reducing the indentation
level below.
> + err = of_phy_register_fixed_link(ofdev->dev.of_node);
> + if (err)
> + goto out_free_fpi;
> +
> + fpi->phy_node = ofdev->dev.of_node;
> + }
> + }
WBR, Sergei
^ permalink raw reply
* Re: [PATCH v5 3/4] drivers/vfio: EEH support for VFIO PCI device
From: Benjamin Herrenschmidt @ 2014-05-21 21:56 UTC (permalink / raw)
To: Alexander Graf
Cc: aik, Gavin Shan, kvm-ppc, alex.williamson, qiudayu, linuxppc-dev
In-Reply-To: <537CA50E.9090404@suse.de>
On Wed, 2014-05-21 at 15:07 +0200, Alexander Graf wrote:
> > +#ifdef CONFIG_VFIO_PCI_EEH
> > +int eeh_vfio_open(struct pci_dev *pdev)
>
> Why vfio? Also that config option will not be set if vfio is compiled as
> a module.
>
> > +{
> > + struct eeh_dev *edev;
> > +
> > + /* No PCI device ? */
> > + if (!pdev)
> > + return -ENODEV;
> > +
> > + /* No EEH device ? */
> > + edev = pci_dev_to_eeh_dev(pdev);
> > + if (!edev || !edev->pe)
> > + return -ENODEV;
> > +
> > + eeh_dev_set_passed(edev, true);
> > + eeh_pe_set_passed(edev->pe, true);
> > +
> > + return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(eeh_vfio_open);
Additionally, shouldn't we have some locking here ? (and in release too)
I don't like relying on the caller locking (if it does it at all).
> > + /* Device existing ? */
> > + ret = eeh_vfio_check_dev(pdev, &edev, &pe);
> > + if (ret) {
> > + pr_debug("%s: Cannot find device %s\n",
> > + __func__, pdev ? pci_name(pdev) : "NULL");
> > + *retval = -7;
>
> What are these? Please use proper kernel internal return values for
> errors. I don't want to see anything even remotely tied to RTAS in any
> of these patches.
Hint: -ENODEV
Cheers,
Ben.
^ permalink raw reply
* Re: [PATCH RESEND net-next 4/9] net: systemport: use the new fixed PHY helpers
From: Sergei Shtylyov @ 2014-05-21 21:55 UTC (permalink / raw)
To: Florian Fainelli, netdev
Cc: Thomas Petazzoni, open list:OPEN FIRMWARE AND..., Aida Mynzhasova,
Richard Cochran, open list, Claudiu Manoil, Vitaly Bordug,
Grant Likely, open list:LINUX FOR POWERPC..., davem
In-Reply-To: <1400708331-18088-5-git-send-email-f.fainelli@gmail.com>
On 05/22/2014 01:38 AM, Florian Fainelli wrote:
> of_phy_connect_fixed_link() is becoming obsolete, and also required
> platform code to register the fixed PHYs at the specified addresses for
> those to be usable. Get rid of it and use the new of_phy_is_fixed_link()
> plus of_phy_register_fixed_link() helpers to transition over the new
> scheme.
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
> drivers/net/ethernet/broadcom/bcmsysport.c | 17 +++++++++++++++--
> drivers/net/ethernet/broadcom/bcmsysport.h | 1 +
> 2 files changed, 16 insertions(+), 2 deletions(-)
> diff --git a/drivers/net/ethernet/broadcom/bcmsysport.c b/drivers/net/ethernet/broadcom/bcmsysport.c
> index d40c5b969e9e..dc708a888f80 100644
> --- a/drivers/net/ethernet/broadcom/bcmsysport.c
> +++ b/drivers/net/ethernet/broadcom/bcmsysport.c
> @@ -1327,8 +1327,8 @@ static int bcm_sysport_open(struct net_device *dev)
> /* Read CRC forward */
> priv->crc_fwd = !!(umac_readl(priv, UMAC_CMD) & CMD_CRC_FWD);
>
> - priv->phydev = of_phy_connect_fixed_link(dev, bcm_sysport_adj_link,
> - priv->phy_interface);
> + priv->phydev = of_phy_connect(dev, priv->phy_dn, bcm_sysport_adj_link,
> + 0, priv->phy_interface);
The continuation line should start on the next character after ( on the
above line, according to the networking coding style.
[...]
WBR, Sergei
^ permalink raw reply
* Re: [PATCH RESEND net-next 1/9] Documentation: devicetree: add old and deprecated 'fixed-link'
From: Sergei Shtylyov @ 2014-05-21 21:48 UTC (permalink / raw)
To: Florian Fainelli, netdev
Cc: Thomas Petazzoni, open list:OPEN FIRMWARE AND..., Aida Mynzhasova,
Richard Cochran, open list, Claudiu Manoil, Vitaly Bordug,
Grant Likely, open list:LINUX FOR POWERPC..., davem
In-Reply-To: <1400708331-18088-2-git-send-email-f.fainelli@gmail.com>
Hello.
On 05/22/2014 01:38 AM, Florian Fainelli wrote:
> Update the fixed-link Device Tree binding documentation to contain
> information about the old and deprecated 5-digit 'fixed-link' property.
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
> Documentation/devicetree/bindings/net/fixed-link.txt | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
> diff --git a/Documentation/devicetree/bindings/net/fixed-link.txt b/Documentation/devicetree/bindings/net/fixed-link.txt
> index e956de1be935..34a991eb213f 100644
> --- a/Documentation/devicetree/bindings/net/fixed-link.txt
> +++ b/Documentation/devicetree/bindings/net/fixed-link.txt
> @@ -18,6 +18,18 @@ properties:
> * 'asym-pause' (boolean, optional), to indicate that asym_pause should
> be enabled.
>
> +Old, deprecated 'fixed-link' binding:
> +
> +* A 'fixed-link' property in the EThernet MAC node, with 5 cells, of the
s/EThernet/Ethernet/
> + form <a b c d e> with the following accepted values:
> + - a: emulated phy id, choose any but but unique to the all specified
s/phy id/PHY ID/
> + fixed-links, from 0 to 31
> + - b: duplex configuration: 0 for half duplex, 1 for full duplex
> + - c: link speed in Mbits/sec, accepted values are: 10, 100 and 1000
> + - d: pause configuration: 0 for no pause, 1 for pause
> + - e: asymetric pause configuration: 0 for no asymetric pause, 1 for asymetric
Asymmetric.
WBR, Sergei
^ permalink raw reply
* [PATCH RESEND net-next 9/9] powerpc/fsl: fsl_soc: remove 'fixed-link' parsing code
From: Florian Fainelli @ 2014-05-21 21:38 UTC (permalink / raw)
To: netdev
Cc: Thomas Petazzoni, open list:OPEN FIRMWARE AND...,
Florian Fainelli, Aida Mynzhasova, Sergei Shtylyov,
Richard Cochran, open list, Claudiu Manoil, Vitaly Bordug,
Grant Likely, open list:LINUX FOR POWERPC..., davem
In-Reply-To: <1400708331-18088-1-git-send-email-f.fainelli@gmail.com>
Parsing and registration of fixed PHY devices was needed with the use of
of_phy_connect_fixed_link() because this function was using the
designated PHY address identifier (first cell of the property) as the
address to bind the PHY on the emulated bus.
Since commit 3be2a49e5c08d268f8af0dd4fe89a24ea8cdc339 ("of: provide a
binding for fixed link PHYs") a new pair of functions has been
introduced which allows for dynamic address allocation of these fixed
PHY devices, but also parses the old 'fixed-link' 5-digit property.
Registration of fixed PHY early in platform code was needed because we
could not issue a fixed MDIO bus re-scan within network drivers. The
fixed PHYs had to be registered before the network drivers would call
of_phy_connect_fixed_link(). All of these caveats are solved now, such
that we can safely remove of_add_fixed_phys() now.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
arch/powerpc/sysdev/fsl_soc.c | 32 --------------------------------
1 file changed, 32 deletions(-)
diff --git a/arch/powerpc/sysdev/fsl_soc.c b/arch/powerpc/sysdev/fsl_soc.c
index 228cf91b91c1..ffd1169ebaab 100644
--- a/arch/powerpc/sysdev/fsl_soc.c
+++ b/arch/powerpc/sysdev/fsl_soc.c
@@ -25,7 +25,6 @@
#include <linux/of.h>
#include <linux/of_platform.h>
#include <linux/phy.h>
-#include <linux/phy_fixed.h>
#include <linux/spi/spi.h>
#include <linux/fsl_devices.h>
#include <linux/fs_enet_pd.h>
@@ -178,37 +177,6 @@ u32 get_baudrate(void)
EXPORT_SYMBOL(get_baudrate);
#endif /* CONFIG_CPM2 */
-#ifdef CONFIG_FIXED_PHY
-static int __init of_add_fixed_phys(void)
-{
- int ret;
- struct device_node *np;
- u32 *fixed_link;
- struct fixed_phy_status status = {};
-
- for_each_node_by_name(np, "ethernet") {
- fixed_link = (u32 *)of_get_property(np, "fixed-link", NULL);
- if (!fixed_link)
- continue;
-
- status.link = 1;
- status.duplex = fixed_link[1];
- status.speed = fixed_link[2];
- status.pause = fixed_link[3];
- status.asym_pause = fixed_link[4];
-
- ret = fixed_phy_add(PHY_POLL, fixed_link[0], &status);
- if (ret) {
- of_node_put(np);
- return ret;
- }
- }
-
- return 0;
-}
-arch_initcall(of_add_fixed_phys);
-#endif /* CONFIG_FIXED_PHY */
-
#if defined(CONFIG_FSL_SOC_BOOKE) || defined(CONFIG_PPC_86xx)
static __be32 __iomem *rstcr;
--
1.9.1
^ permalink raw reply related
* [PATCH RESEND net-next 8/9] of: mdio: remove of_phy_connect_fixed_link
From: Florian Fainelli @ 2014-05-21 21:38 UTC (permalink / raw)
To: netdev
Cc: Thomas Petazzoni, open list:OPEN FIRMWARE AND...,
Florian Fainelli, Aida Mynzhasova, Sergei Shtylyov,
Richard Cochran, open list, Claudiu Manoil, Vitaly Bordug,
Grant Likely, open list:LINUX FOR POWERPC..., davem
In-Reply-To: <1400708331-18088-1-git-send-email-f.fainelli@gmail.com>
All in-tree drivers have been converted to use the new pair of
functions: of_is_fixed_phy_link() plus of_phy_register_fixed_link(), we
can now safely remove of_phy_connect_fixed_link.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/of/of_mdio.c | 38 --------------------------------------
include/linux/of_mdio.h | 10 ----------
2 files changed, 48 deletions(-)
diff --git a/drivers/of/of_mdio.c b/drivers/of/of_mdio.c
index 1def0bb5cb37..4c1e01ed16dc 100644
--- a/drivers/of/of_mdio.c
+++ b/drivers/of/of_mdio.c
@@ -246,44 +246,6 @@ struct phy_device *of_phy_connect(struct net_device *dev,
EXPORT_SYMBOL(of_phy_connect);
/**
- * of_phy_connect_fixed_link - Parse fixed-link property and return a dummy phy
- * @dev: pointer to net_device claiming the phy
- * @hndlr: Link state callback for the network device
- * @iface: PHY data interface type
- *
- * This function is a temporary stop-gap and will be removed soon. It is
- * only to support the fs_enet, ucc_geth and gianfar Ethernet drivers. Do
- * not call this function from new drivers.
- */
-struct phy_device *of_phy_connect_fixed_link(struct net_device *dev,
- void (*hndlr)(struct net_device *),
- phy_interface_t iface)
-{
- struct device_node *net_np;
- char bus_id[MII_BUS_ID_SIZE + 3];
- struct phy_device *phy;
- const __be32 *phy_id;
- int sz;
-
- if (!dev->dev.parent)
- return NULL;
-
- net_np = dev->dev.parent->of_node;
- if (!net_np)
- return NULL;
-
- phy_id = of_get_property(net_np, "fixed-link", &sz);
- if (!phy_id || sz < sizeof(*phy_id))
- return NULL;
-
- sprintf(bus_id, PHY_ID_FMT, "fixed-0", be32_to_cpu(phy_id[0]));
-
- phy = phy_connect(dev, bus_id, hndlr, iface);
- return IS_ERR(phy) ? NULL : phy;
-}
-EXPORT_SYMBOL(of_phy_connect_fixed_link);
-
-/**
* of_phy_attach - Attach to a PHY without starting the state machine
* @dev: pointer to net_device claiming the phy
* @phy_np: Node pointer for the PHY
diff --git a/include/linux/of_mdio.h b/include/linux/of_mdio.h
index 0aa367e316cb..d449018d0726 100644
--- a/include/linux/of_mdio.h
+++ b/include/linux/of_mdio.h
@@ -22,9 +22,6 @@ extern struct phy_device *of_phy_connect(struct net_device *dev,
struct phy_device *of_phy_attach(struct net_device *dev,
struct device_node *phy_np, u32 flags,
phy_interface_t iface);
-extern struct phy_device *of_phy_connect_fixed_link(struct net_device *dev,
- void (*hndlr)(struct net_device *),
- phy_interface_t iface);
extern struct mii_bus *of_mdio_find_bus(struct device_node *mdio_np);
@@ -59,13 +56,6 @@ static inline struct phy_device *of_phy_attach(struct net_device *dev,
return NULL;
}
-static inline struct phy_device *of_phy_connect_fixed_link(struct net_device *dev,
- void (*hndlr)(struct net_device *),
- phy_interface_t iface)
-{
- return NULL;
-}
-
static inline struct mii_bus *of_mdio_find_bus(struct device_node *mdio_np)
{
return NULL;
--
1.9.1
^ permalink raw reply related
* [PATCH RESEND net-next 7/9] ucc_geth: use the new fixed PHY helpers
From: Florian Fainelli @ 2014-05-21 21:38 UTC (permalink / raw)
To: netdev
Cc: Thomas Petazzoni, open list:OPEN FIRMWARE AND...,
Florian Fainelli, Aida Mynzhasova, Sergei Shtylyov,
Richard Cochran, open list, Claudiu Manoil, Vitaly Bordug,
Grant Likely, open list:LINUX FOR POWERPC..., davem
In-Reply-To: <1400708331-18088-1-git-send-email-f.fainelli@gmail.com>
of_phy_connect_fixed_link() is becoming obsolete, and also required
platform code to register the fixed PHYs at the specified addresses for
those to be usable. Get rid of it and use the new of_phy_is_fixed_link()
plus of_phy_register_fixed_link() helpers to transition over the new
scheme.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/net/ethernet/freescale/ucc_geth.c | 14 +++++++++++---
1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/freescale/ucc_geth.c b/drivers/net/ethernet/freescale/ucc_geth.c
index c8299c31b21f..fab39e295441 100644
--- a/drivers/net/ethernet/freescale/ucc_geth.c
+++ b/drivers/net/ethernet/freescale/ucc_geth.c
@@ -1728,9 +1728,6 @@ static int init_phy(struct net_device *dev)
phydev = of_phy_connect(dev, ug_info->phy_node, &adjust_link, 0,
priv->phy_interface);
- if (!phydev)
- phydev = of_phy_connect_fixed_link(dev, &adjust_link,
- priv->phy_interface);
if (!phydev) {
dev_err(&dev->dev, "Could not attach to PHY\n");
return -ENODEV;
@@ -3790,6 +3787,17 @@ static int ucc_geth_probe(struct platform_device* ofdev)
ug_info->uf_info.irq = irq_of_parse_and_map(np, 0);
ug_info->phy_node = of_parse_phandle(np, "phy-handle", 0);
+ if (!ug_info->phy_node) {
+ /* In the case of a fixed PHY, the DT node associated
+ * to the PHY is the Ethernet MAC DT node.
+ */
+ if (of_phy_is_fixed_link(np)) {
+ err = of_phy_register_fixed_link(np);
+ if (err)
+ return err;
+ }
+ ug_info->phy_node = np;
+ }
/* Find the TBI PHY node. If it's not there, we don't support SGMII */
ug_info->tbi_node = of_parse_phandle(np, "tbi-handle", 0);
--
1.9.1
^ permalink raw reply related
* [PATCH RESEND net-next 6/9] gianfar: use the new fixed PHY helpers
From: Florian Fainelli @ 2014-05-21 21:38 UTC (permalink / raw)
To: netdev
Cc: Thomas Petazzoni, open list:OPEN FIRMWARE AND...,
Florian Fainelli, Aida Mynzhasova, Sergei Shtylyov,
Richard Cochran, open list, Claudiu Manoil, Vitaly Bordug,
Grant Likely, open list:LINUX FOR POWERPC..., davem
In-Reply-To: <1400708331-18088-1-git-send-email-f.fainelli@gmail.com>
of_phy_connect_fixed_link() is becoming obsolete, and also required
platform code to register the fixed PHYs at the specified addresses for
those to be usable. Get rid of it and use the new of_phy_is_fixed_link()
plus of_phy_register_fixed_link() helpers to transition over the new
scheme.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/net/ethernet/freescale/gianfar.c | 14 +++++++++++---
1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/freescale/gianfar.c b/drivers/net/ethernet/freescale/gianfar.c
index e2d42475b006..282674027c92 100644
--- a/drivers/net/ethernet/freescale/gianfar.c
+++ b/drivers/net/ethernet/freescale/gianfar.c
@@ -889,6 +889,17 @@ static int gfar_of_init(struct platform_device *ofdev, struct net_device **pdev)
priv->phy_node = of_parse_phandle(np, "phy-handle", 0);
+ /* In the case of a fixed PHY, the DT node associated
+ * to the PHY is the Ethernet MAC DT node.
+ */
+ if (of_phy_is_fixed_link(np)) {
+ err = of_phy_register_fixed_link(np);
+ if (err)
+ goto err_grp_init;
+
+ priv->phy_node = np;
+ }
+
/* Find the TBI PHY. If it's not there, we don't support SGMII */
priv->tbi_node = of_parse_phandle(np, "tbi-handle", 0);
@@ -1660,9 +1671,6 @@ static int init_phy(struct net_device *dev)
priv->phydev = of_phy_connect(dev, priv->phy_node, &adjust_link, 0,
interface);
- if (!priv->phydev)
- priv->phydev = of_phy_connect_fixed_link(dev, &adjust_link,
- interface);
if (!priv->phydev) {
dev_err(&dev->dev, "could not attach to PHY\n");
return -ENODEV;
--
1.9.1
^ permalink raw reply related
* [PATCH RESEND net-next 5/9] fs_enet: use the new fixed PHY helpers
From: Florian Fainelli @ 2014-05-21 21:38 UTC (permalink / raw)
To: netdev
Cc: Thomas Petazzoni, open list:OPEN FIRMWARE AND...,
Florian Fainelli, Aida Mynzhasova, Sergei Shtylyov,
Richard Cochran, open list, Claudiu Manoil, Vitaly Bordug,
Grant Likely, open list:LINUX FOR POWERPC..., davem
In-Reply-To: <1400708331-18088-1-git-send-email-f.fainelli@gmail.com>
of_phy_connect_fixed_link() is becoming obsolete, and also required
platform code to register the fixed PHYs at the specified addresses for
those to be usable. Get rid of it and use the new of_phy_is_fixed_link()
plus of_phy_register_fixed_link() helpers to transition over the new
scheme.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c | 16 +++++++++-------
1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c b/drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c
index dc80db41d6b3..d602711e00e9 100644
--- a/drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c
+++ b/drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c
@@ -792,10 +792,6 @@ static int fs_init_phy(struct net_device *dev)
phydev = of_phy_connect(dev, fep->fpi->phy_node, &fs_adjust_link, 0,
iface);
if (!phydev) {
- phydev = of_phy_connect_fixed_link(dev, &fs_adjust_link,
- iface);
- }
- if (!phydev) {
dev_err(&dev->dev, "Could not attach to PHY\n");
return -ENODEV;
}
@@ -1029,9 +1025,15 @@ static int fs_enet_probe(struct platform_device *ofdev)
fpi->use_napi = 1;
fpi->napi_weight = 17;
fpi->phy_node = of_parse_phandle(ofdev->dev.of_node, "phy-handle", 0);
- if ((!fpi->phy_node) && (!of_get_property(ofdev->dev.of_node, "fixed-link",
- NULL)))
- goto out_free_fpi;
+ if (!fpi->phy_node) {
+ if (of_phy_is_fixed_link(ofdev->dev.of_node)) {
+ err = of_phy_register_fixed_link(ofdev->dev.of_node);
+ if (err)
+ goto out_free_fpi;
+
+ fpi->phy_node = ofdev->dev.of_node;
+ }
+ }
if (of_device_is_compatible(ofdev->dev.of_node, "fsl,mpc5125-fec")) {
phy_connection_type = of_get_property(ofdev->dev.of_node,
--
1.9.1
^ permalink raw reply related
* [PATCH RESEND net-next 4/9] net: systemport: use the new fixed PHY helpers
From: Florian Fainelli @ 2014-05-21 21:38 UTC (permalink / raw)
To: netdev
Cc: Thomas Petazzoni, open list:OPEN FIRMWARE AND...,
Florian Fainelli, Aida Mynzhasova, Sergei Shtylyov,
Richard Cochran, open list, Claudiu Manoil, Vitaly Bordug,
Grant Likely, open list:LINUX FOR POWERPC..., davem
In-Reply-To: <1400708331-18088-1-git-send-email-f.fainelli@gmail.com>
of_phy_connect_fixed_link() is becoming obsolete, and also required
platform code to register the fixed PHYs at the specified addresses for
those to be usable. Get rid of it and use the new of_phy_is_fixed_link()
plus of_phy_register_fixed_link() helpers to transition over the new
scheme.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/net/ethernet/broadcom/bcmsysport.c | 17 +++++++++++++++--
drivers/net/ethernet/broadcom/bcmsysport.h | 1 +
2 files changed, 16 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bcmsysport.c b/drivers/net/ethernet/broadcom/bcmsysport.c
index d40c5b969e9e..dc708a888f80 100644
--- a/drivers/net/ethernet/broadcom/bcmsysport.c
+++ b/drivers/net/ethernet/broadcom/bcmsysport.c
@@ -1327,8 +1327,8 @@ static int bcm_sysport_open(struct net_device *dev)
/* Read CRC forward */
priv->crc_fwd = !!(umac_readl(priv, UMAC_CMD) & CMD_CRC_FWD);
- priv->phydev = of_phy_connect_fixed_link(dev, bcm_sysport_adj_link,
- priv->phy_interface);
+ priv->phydev = of_phy_connect(dev, priv->phy_dn, bcm_sysport_adj_link,
+ 0, priv->phy_interface);
if (!priv->phydev) {
netdev_err(dev, "could not attach to PHY\n");
return -ENODEV;
@@ -1551,6 +1551,19 @@ static int bcm_sysport_probe(struct platform_device *pdev)
if (priv->phy_interface < 0)
priv->phy_interface = PHY_INTERFACE_MODE_GMII;
+ /* In the case of a fixed PHY, the DT node associated
+ * to the PHY is the Ethernet MAC DT node.
+ */
+ if (of_phy_is_fixed_link(dn)) {
+ ret = of_phy_register_fixed_link(dn);
+ if (ret) {
+ dev_err(&pdev->dev, "failed to register fixed PHY\n");
+ goto err;
+ }
+
+ priv->phy_dn = dn;
+ }
+
/* Initialize netdevice members */
macaddr = of_get_mac_address(dn);
if (!macaddr || !is_valid_ether_addr(macaddr)) {
diff --git a/drivers/net/ethernet/broadcom/bcmsysport.h b/drivers/net/ethernet/broadcom/bcmsysport.h
index abdeb62616df..73fd04a94797 100644
--- a/drivers/net/ethernet/broadcom/bcmsysport.h
+++ b/drivers/net/ethernet/broadcom/bcmsysport.h
@@ -656,6 +656,7 @@ struct bcm_sysport_priv {
unsigned int rx_c_index;
/* PHY device */
+ struct device_node *phy_dn;
struct phy_device *phydev;
phy_interface_t phy_interface;
int old_pause;
--
1.9.1
^ permalink raw reply related
* [PATCH RESEND net-next 3/9] net: bcmgenet: use the new fixed PHY helpers
From: Florian Fainelli @ 2014-05-21 21:38 UTC (permalink / raw)
To: netdev
Cc: Thomas Petazzoni, open list:OPEN FIRMWARE AND...,
Florian Fainelli, Aida Mynzhasova, Sergei Shtylyov,
Richard Cochran, open list, Claudiu Manoil, Vitaly Bordug,
Grant Likely, open list:LINUX FOR POWERPC..., davem
In-Reply-To: <1400708331-18088-1-git-send-email-f.fainelli@gmail.com>
of_phy_connect_fixed_link() is becoming obsolete, and also required
platform code to register the fixed PHYs at the specified addresses for
those to be usable. Get rid of it and use the new of_phy_is_fixed_link()
plus of_phy_register_fixed_link() helpers to transition over the new
scheme.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/net/ethernet/broadcom/genet/bcmmii.c | 21 +++++++++++++--------
1 file changed, 13 insertions(+), 8 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/genet/bcmmii.c b/drivers/net/ethernet/broadcom/genet/bcmmii.c
index 4608673beaff..add8d8596084 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmmii.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmmii.c
@@ -298,6 +298,7 @@ int bcmgenet_mii_config(struct net_device *dev)
static int bcmgenet_mii_probe(struct net_device *dev)
{
struct bcmgenet_priv *priv = netdev_priv(dev);
+ struct device_node *dn = priv->pdev->dev.of_node;
struct phy_device *phydev;
unsigned int phy_flags;
int ret;
@@ -307,15 +308,19 @@ static int bcmgenet_mii_probe(struct net_device *dev)
return 0;
}
- if (priv->phy_dn)
- phydev = of_phy_connect(dev, priv->phy_dn,
- bcmgenet_mii_setup, 0,
- priv->phy_interface);
- else
- phydev = of_phy_connect_fixed_link(dev,
- bcmgenet_mii_setup,
- priv->phy_interface);
+ /* In the case of a fixed PHY, the DT node associated
+ * to the PHY is the Ethernet MAC DT node.
+ */
+ if (of_phy_is_fixed_link(dn)) {
+ ret = of_phy_register_fixed_link(dn);
+ if (ret)
+ return ret;
+
+ priv->phy_dn = dn;
+ }
+ phydev = of_phy_connect(dev, priv->phy_dn, bcmgenet_mii_setup, 0,
+ priv->phy_interface);
if (!phydev) {
pr_err("could not attach to PHY\n");
return -ENODEV;
--
1.9.1
^ permalink raw reply related
* [PATCH RESEND net-next 2/9] Documentation: devicetree: net: refer to fixed-link.txt
From: Florian Fainelli @ 2014-05-21 21:38 UTC (permalink / raw)
To: netdev
Cc: Thomas Petazzoni, open list:OPEN FIRMWARE AND...,
Florian Fainelli, Aida Mynzhasova, Sergei Shtylyov,
Richard Cochran, open list, Claudiu Manoil, Vitaly Bordug,
Grant Likely, open list:LINUX FOR POWERPC..., davem
In-Reply-To: <1400708331-18088-1-git-send-email-f.fainelli@gmail.com>
Update the Freescale TSEC PHY, Broadcom GENET & SYSTEMPORT Device Tree
binding documentation to refer to the fixed-link Device Tree binding in
fixed-link.txt.
Reviewed-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
Documentation/devicetree/bindings/net/broadcom-bcmgenet.txt | 2 +-
Documentation/devicetree/bindings/net/broadcom-systemport.txt | 2 +-
Documentation/devicetree/bindings/net/fsl-tsec-phy.txt | 5 +----
3 files changed, 3 insertions(+), 6 deletions(-)
diff --git a/Documentation/devicetree/bindings/net/broadcom-bcmgenet.txt b/Documentation/devicetree/bindings/net/broadcom-bcmgenet.txt
index f2febb94550e..451fef26b4df 100644
--- a/Documentation/devicetree/bindings/net/broadcom-bcmgenet.txt
+++ b/Documentation/devicetree/bindings/net/broadcom-bcmgenet.txt
@@ -24,7 +24,7 @@ Optional properties:
- fixed-link: When the GENET interface is connected to a MoCA hardware block or
when operating in a RGMII to RGMII type of connection, or when the MDIO bus is
voluntarily disabled, this property should be used to describe the "fixed link".
- See Documentation/devicetree/bindings/net/fsl-tsec-phy.txt for information on
+ See Documentation/devicetree/bindings/net/fixed-link.txt for information on
the property specifics
Required child nodes:
diff --git a/Documentation/devicetree/bindings/net/broadcom-systemport.txt b/Documentation/devicetree/bindings/net/broadcom-systemport.txt
index 1b7600e022dd..c183ea90d9bc 100644
--- a/Documentation/devicetree/bindings/net/broadcom-systemport.txt
+++ b/Documentation/devicetree/bindings/net/broadcom-systemport.txt
@@ -8,7 +8,7 @@ Required properties:
- local-mac-address: Ethernet MAC address (48 bits) of this adapter
- phy-mode: Should be a string describing the PHY interface to the
Ethernet switch/PHY, see Documentation/devicetree/bindings/net/ethernet.txt
-- fixed-link: see Documentation/devicetree/bindings/net/fsl-tsec-phy.txt for
+- fixed-link: see Documentation/devicetree/bindings/net/fixed-link.txt for
the property specific details
Optional properties:
diff --git a/Documentation/devicetree/bindings/net/fsl-tsec-phy.txt b/Documentation/devicetree/bindings/net/fsl-tsec-phy.txt
index 737cdef4f903..be6ea8960f20 100644
--- a/Documentation/devicetree/bindings/net/fsl-tsec-phy.txt
+++ b/Documentation/devicetree/bindings/net/fsl-tsec-phy.txt
@@ -42,10 +42,7 @@ Properties:
interrupt. For TSEC and eTSEC devices, the first interrupt is
transmit, the second is receive, and the third is error.
- phy-handle : See ethernet.txt file in the same directory.
- - fixed-link : <a b c d e> where a is emulated phy id - choose any,
- but unique to the all specified fixed-links, b is duplex - 0 half,
- 1 full, c is link speed - d#10/d#100/d#1000, d is pause - 0 no
- pause, 1 pause, e is asym_pause - 0 no asym_pause, 1 asym_pause.
+ - fixed-link : See fixed-link.txt in the same directory.
- phy-connection-type : See ethernet.txt file in the same directory.
This property is only really needed if the connection is of type
"rgmii-id", as all other connection types are detected by hardware.
--
1.9.1
^ permalink raw reply related
* [PATCH RESEND net-next 1/9] Documentation: devicetree: add old and deprecated 'fixed-link'
From: Florian Fainelli @ 2014-05-21 21:38 UTC (permalink / raw)
To: netdev
Cc: Thomas Petazzoni, open list:OPEN FIRMWARE AND...,
Florian Fainelli, Aida Mynzhasova, Sergei Shtylyov,
Richard Cochran, open list, Claudiu Manoil, Vitaly Bordug,
Grant Likely, open list:LINUX FOR POWERPC..., davem
In-Reply-To: <1400708331-18088-1-git-send-email-f.fainelli@gmail.com>
Update the fixed-link Device Tree binding documentation to contain
information about the old and deprecated 5-digit 'fixed-link' property.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
Documentation/devicetree/bindings/net/fixed-link.txt | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/Documentation/devicetree/bindings/net/fixed-link.txt b/Documentation/devicetree/bindings/net/fixed-link.txt
index e956de1be935..34a991eb213f 100644
--- a/Documentation/devicetree/bindings/net/fixed-link.txt
+++ b/Documentation/devicetree/bindings/net/fixed-link.txt
@@ -18,6 +18,18 @@ properties:
* 'asym-pause' (boolean, optional), to indicate that asym_pause should
be enabled.
+Old, deprecated 'fixed-link' binding:
+
+* A 'fixed-link' property in the EThernet MAC node, with 5 cells, of the
+ form <a b c d e> with the following accepted values:
+ - a: emulated phy id, choose any but but unique to the all specified
+ fixed-links, from 0 to 31
+ - b: duplex configuration: 0 for half duplex, 1 for full duplex
+ - c: link speed in Mbits/sec, accepted values are: 10, 100 and 1000
+ - d: pause configuration: 0 for no pause, 1 for pause
+ - e: asymetric pause configuration: 0 for no asymetric pause, 1 for asymetric
+ pause
+
Example:
ethernet@0 {
--
1.9.1
^ permalink raw reply related
* [PATCH RESEND 0/9 net-next] net: of_phy_connect_fixed_link removal
From: Florian Fainelli @ 2014-05-21 21:38 UTC (permalink / raw)
To: netdev
Cc: Thomas Petazzoni, open list:OPEN FIRMWARE AND...,
Florian Fainelli, Aida Mynzhasova, Sergei Shtylyov,
Richard Cochran, open list, Claudiu Manoil, Vitaly Bordug,
Grant Likely, open list:LINUX FOR POWERPC..., davem
Hi all,
This patch set removes of_phy_connect_fixed_link() from the tree now that
we have a better solution for dealing with fixed PHY (emulated PHY) devices
for drivers that require them.
First two patches update the 'fixed-link' Device Tree binding and drivers to
refere to it.
Patches 3 to 7 update the in-tree network drivers that use
of_phy_connect_fixed_link()
Patch 8 removes of_phy_connect_fixed_link
Patch 9 removes the PowerPC code that parsed the 'fixed-link' property.
Patch 9 can be merged via the net-next tree if the PowerPC folks ack it,
but it really has to be merged after the first 8 patches in order to avoid
breakage.
Florian Fainelli (9):
Documentation: devicetree: add old and deprecated 'fixed-link'
Documentation: devicetree: net: refer to fixed-link.txt
net: bcmgenet: use the new fixed PHY helpers
net: systemport: use the new fixed PHY helpers
fs_enet: use the new fixed PHY helpers
gianfar: use the new fixed PHY helpers
ucc_geth: use the new fixed PHY helpers
of: mdio: remove of_phy_connect_fixed_link
powerpc/fsl: fsl_soc: remove 'fixed-link' parsing code
.../devicetree/bindings/net/broadcom-bcmgenet.txt | 2 +-
.../bindings/net/broadcom-systemport.txt | 2 +-
.../devicetree/bindings/net/fixed-link.txt | 12 +++++++
.../devicetree/bindings/net/fsl-tsec-phy.txt | 5 +--
arch/powerpc/sysdev/fsl_soc.c | 32 ------------------
drivers/net/ethernet/broadcom/bcmsysport.c | 17 ++++++++--
drivers/net/ethernet/broadcom/bcmsysport.h | 1 +
drivers/net/ethernet/broadcom/genet/bcmmii.c | 21 +++++++-----
.../net/ethernet/freescale/fs_enet/fs_enet-main.c | 16 +++++----
drivers/net/ethernet/freescale/gianfar.c | 14 ++++++--
drivers/net/ethernet/freescale/ucc_geth.c | 14 ++++++--
drivers/of/of_mdio.c | 38 ----------------------
include/linux/of_mdio.h | 10 ------
13 files changed, 75 insertions(+), 109 deletions(-)
--
1.9.1
^ permalink raw reply
* Re: [PATCH 1/2] mtd/spi: support en25s64 device
From: Scott Wood @ 2014-05-21 20:50 UTC (permalink / raw)
To: Shengzhou Liu; +Cc: linuxppc-dev
In-Reply-To: <1400666751-11459-1-git-send-email-Shengzhou.Liu@freescale.com>
On Wed, 2014-05-21 at 18:05 +0800, Shengzhou Liu wrote:
> Add support for EON en25s64 spi device.
>
> Signed-off-by: Shengzhou Liu <Shengzhou.Liu@freescale.com>
> ---
> drivers/mtd/devices/m25p80.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
> index 7eda71d..6989311 100644
> --- a/drivers/mtd/devices/m25p80.c
> +++ b/drivers/mtd/devices/m25p80.c
> @@ -745,6 +745,7 @@ static const struct spi_device_id m25p_ids[] = {
> { "en25q32b", INFO(0x1c3016, 0, 64 * 1024, 64, 0) },
> { "en25p64", INFO(0x1c2017, 0, 64 * 1024, 128, 0) },
> { "en25q64", INFO(0x1c3017, 0, 64 * 1024, 128, SECT_4K) },
> + { "en25s64", INFO(0x1c3817, 0, 64 * 1024, 128, 0) },
> { "en25qh256", INFO(0x1c7019, 0, 64 * 1024, 512, 0) },
>
> /* ESMT */
This needs to be sent to the mtd and/or spi maintainers, not here.
What does this have to do with patch 2/2? Don't put unrelated things in
the same patchset, especially when they're destined for different
maintainers.
-Scott
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox