Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1735056

Re: [PATCH v4 01/18] fpga: bridge: support getting bridge from device

Show all headers | View raw

On Thu, Sep 14, 2017 at 5:54 PM, Moritz Fischer <mdf@kernel.org> wrote:
> On Thu, Sep 14, 2017 at 03:29:09PM -0700, matthew.gerlach@linux.intel.com wrote:
>>
>>
>> On Thu, 14 Sep 2017, Alan Tull wrote:
>>
>> > On Wed, Sep 13, 2017 at 6:38 PM,  <matthew.gerlach@linux.intel.com> wrote:
>> >
>> > Hi Matthew,
>> >
>> > >
>> > > Hi Alan,
>> > >
>> > > Two minor nits below.
>> > >
>> > > Matthew Gerlach
>> > >
>> > > On Wed, 13 Sep 2017, Alan Tull wrote:
>> > >
>> > > > Add two functions for getting the FPGA bridge from the device
>> > > > rather than device tree node.  This is to enable writing code
>> > > > that will support using FPGA bridges without device tree.
>> > > > Rename one old function to make it clear that it is device
>> > > > tree-ish.  This leaves us with 3 functions for getting a bridge:
>> > > >
>> > > > * fpga_bridge_get
>> > > >  Get the bridge given the device.
>> > > >
>> > > > * fpga_bridges_get_to_list
>> > > >  Given the device, get the bridge and add it to a list.
>> > > >
>> > > > * of_fpga_bridges_get_to_list
>> > > >  Renamed from priviously existing fpga_bridges_get_to_list.
>> > > >  Given the device node, get the bridge and add it to a list.
>> > > >
>> > > > Signed-off-by: Alan Tull <atull@kernel.org>
>> > > > ---
>> > > > v2: use list_for_each_entry
>> > > >    static the bridge_list_lock
>> > > >    update copyright and author email
>> > > > v3: no change to this patch in this version of patchset
>> > > > v4: no change to this patch in this version of patchset
>> > > > ---
>> > > > drivers/fpga/fpga-bridge.c       | 110
>> > > > +++++++++++++++++++++++++++++++--------
>> > > > drivers/fpga/fpga-region.c       |  11 ++--
>> > > > include/linux/fpga/fpga-bridge.h |   7 ++-
>> > > > 3 files changed, 100 insertions(+), 28 deletions(-)
>> > > >
>> > > > diff --git a/drivers/fpga/fpga-bridge.c b/drivers/fpga/fpga-bridge.c
>> > > > index fcd2bd3..af6d97e 100644
>> > > > --- a/drivers/fpga/fpga-bridge.c
>> > > > +++ b/drivers/fpga/fpga-bridge.c
>> > > > @@ -2,6 +2,7 @@
>> > > >  * FPGA Bridge Framework Driver
>> > > >  *
>> > > >  *  Copyright (C) 2013-2016 Altera Corporation, All Rights Reserved.
>> > > > + *  Copyright (C) 2017 Intel Corporation
>> > > >  *
>> > > >  * This program is free software; you can redistribute it and/or modify it
>> > > >  * under the terms and conditions of the GNU General Public License,
>> > > > @@ -70,29 +71,12 @@ int fpga_bridge_disable(struct fpga_bridge *bridge)
>> > > > }
>> > > > EXPORT_SYMBOL_GPL(fpga_bridge_disable);
>> > > >
>> > > > -/**
>> > > > - * of_fpga_bridge_get - get an exclusive reference to a fpga bridge
>> > > > - *
>> > > > - * @np: node pointer of a FPGA bridge
>> > > > - * @info: fpga image specific information
>> > > > - *
>> > > > - * Return fpga_bridge struct if successful.
>> > > > - * Return -EBUSY if someone already has a reference to the bridge.
>> > > > - * Return -ENODEV if @np is not a FPGA Bridge.
>> > > > - */
>> > > > -struct fpga_bridge *of_fpga_bridge_get(struct device_node *np,
>> > > > -                                      struct fpga_image_info *info)
>> > > > -
>> > > > +struct fpga_bridge *__fpga_bridge_get(struct device *dev,
>> > > > +                                     struct fpga_image_info *info)
>> > >
>> > >
>> > > Should this be a static function?
>> >
>> > You are right.  Will fix in v5.
>> >
>> > >
>> > > I was recently told by mtd maintainers that function names prefixed with
>> > > __ should be avoided.
>> >
>> > I see functions named thusly around in the kernel.   Can you point me
>> > to that thread or let me know what their thinking was about this?  I
>> > am open for suggestions for a new function name.
>>
>> Marek Vasut just told me to "Avoid function names with __ prefix" in
>>
>> https://patchwork.kernel.org/patch/9883977/
>>
>> I tend to agree with you that the __ prefix seems to be around, but
>> this could be a case of "evolution" of coding style.  Historically,
>> __ seems to mean an internal version of an external function.  If __
>> is to be avoided, we might have to rename to fpga_bridge_get_internal().
>
> Bear in mind that coding style can be subsystem specific. What applies
> to MTD doesn't necessarily have to apply here.

Yes I agree.  I'll keep the __ name unless someone gives us good
explanation of the naming practice they are recommending.  But yes, it
should have been static.

Alan

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCH v4 00/18]  Enable upper layers using FPGA region w/o device tree Alan Tull <atull@kernel.org> - 2017-09-13 22:50 +0200
  [PATCH v4 10/18] fpga: region: separate out code that parses the overlay Alan Tull <atull@kernel.org> - 2017-09-13 22:50 +0200
  [PATCH v4 18/18] fpga: add attribute groups Alan Tull <atull@kernel.org> - 2017-09-13 22:50 +0200
  [PATCH v4 16/18] fpga: of-fpga-region: accept overlays that don't program FPGA Alan Tull <atull@kernel.org> - 2017-09-13 22:50 +0200
  [PATCH v4 02/18] fpga: mgr: API change to replace fpga load functions with single function Alan Tull <atull@kernel.org> - 2017-09-13 22:50 +0200
  [PATCH v4 17/18] fpga: clean up fpga Kconfig Alan Tull <atull@kernel.org> - 2017-09-13 22:50 +0200
    Re: [PATCH v4 17/18] fpga: clean up fpga Kconfig matthew.gerlach@linux.intel.com - 2017-09-14 18:00 +0200
      Re: [PATCH v4 17/18] fpga: clean up fpga Kconfig Alan Tull <atull@kernel.org> - 2017-09-14 22:50 +0200
  [PATCH v4 06/18] fpga: region: get mgr early on Alan Tull <atull@kernel.org> - 2017-09-13 23:00 +0200
  [PATCH v4 14/18] fpga: region: add fpga_region_class_find Alan Tull <atull@kernel.org> - 2017-09-13 23:00 +0200
  [PATCH v4 05/18] fpga: region: remove unneeded of_node_get and put Alan Tull <atull@kernel.org> - 2017-09-13 23:00 +0200
  [PATCH v4 13/18] fpga: region: add register/unregister functions Alan Tull <atull@kernel.org> - 2017-09-13 23:00 +0200
    Re: [PATCH v4 13/18] fpga: region: add register/unregister functions Wu Hao <hao.wu@intel.com> - 2017-09-14 12:10 +0200
      Re: [PATCH v4 13/18] fpga: region: add register/unregister functions Alan Tull <atull@kernel.org> - 2017-09-14 22:10 +0200
  [PATCH v4 11/18] fpga: region: add fpga-region.h header Alan Tull <atull@kernel.org> - 2017-09-13 23:00 +0200
    Re: [PATCH v4 11/18] fpga: region: add fpga-region.h header Wu Hao <hao.wu@intel.com> - 2017-09-14 12:00 +0200
      Re: [PATCH v4 11/18] fpga: region: add fpga-region.h header Alan Tull <atull@kernel.org> - 2017-09-14 21:40 +0200
  [PATCH v4 15/18] fpga: region: move device tree support to of-fpga-region.c Alan Tull <atull@kernel.org> - 2017-09-13 23:00 +0200
    Re: [PATCH v4 15/18] fpga: region: move device tree support to  of-fpga-region.c matthew.gerlach@linux.intel.com - 2017-09-14 18:00 +0200
  [PATCH v4 03/18] fpga: mgr: separate getting/locking FPGA manager Alan Tull <atull@kernel.org> - 2017-09-13 23:00 +0200
  [PATCH v4 09/18] fpga: region: use image info as parameter for programming region Alan Tull <atull@kernel.org> - 2017-09-13 23:00 +0200
  [PATCH v4 01/18] fpga: bridge: support getting bridge from device Alan Tull <atull@kernel.org> - 2017-09-13 23:00 +0200
    Re: [PATCH v4 01/18] fpga: bridge: support getting bridge from  device matthew.gerlach@linux.intel.com - 2017-09-14 01:40 +0200
      Re: [PATCH v4 01/18] fpga: bridge: support getting bridge from device Alan Tull <atull@kernel.org> - 2017-09-14 21:30 +0200
        Re: [PATCH v4 01/18] fpga: bridge: support getting bridge from  device matthew.gerlach@linux.intel.com - 2017-09-15 00:30 +0200
          Re: [PATCH v4 01/18] fpga: bridge: support getting bridge from device Moritz Fischer <mdf@kernel.org> - 2017-09-15 01:00 +0200
            Re: [PATCH v4 01/18] fpga: bridge: support getting bridge from device Alan Tull <atull@kernel.org> - 2017-09-19 18:10 +0200
    Re: [PATCH v4 01/18] fpga: bridge: support getting bridge from device Moritz Fischer <mdf@kernel.org> - 2017-09-18 20:00 +0200
      Re: [PATCH v4 01/18] fpga: bridge: support getting bridge from device Alan Tull <atull@kernel.org> - 2017-09-18 23:00 +0200
    Re: [PATCH v4 01/18] fpga: bridge: support getting bridge from device Moritz Fischer <mdf@kernel.org> - 2017-09-19 01:00 +0200
      Re: [PATCH v4 01/18] fpga: bridge: support getting bridge from device Alan Tull <atull@kernel.org> - 2017-09-19 17:40 +0200
  [PATCH v4 12/18] fpga: region: rename some functions prior to moving Alan Tull <atull@kernel.org> - 2017-09-13 23:00 +0200
  [PATCH v4 07/18] fpga: region: check for child regions before allocing image info Alan Tull <atull@kernel.org> - 2017-09-13 23:00 +0200
  [PATCH v4 08/18] fpga: region: fix slow warning with more than one overlay Alan Tull <atull@kernel.org> - 2017-09-13 23:00 +0200

csiph-web