[PATCH RTEMS 3/7] libfreebsd: Added ofw_if.h

Wed Jul 29 19:36:55 UTC 2020

On Wed, Jul 29, 2020 at 12:45 PM Niteesh G. S. <niteesh.gs at gmail.com> wrote:
>
> On Wed, Jul 29, 2020 at 9:29 PM Gedare Bloom <gedare at rtems.org> wrote:
>>
>> On Wed, Jul 15, 2020 at 12:32 AM G S Niteesh Babu <niteesh.gs at gmail.com> wrote:
>> >
>> > This file is the RTEMS implementation of ofw_if.h in FreeBSD. The
>> > ofw_if.h in FreeBSD is an autogenerated header file that maps the
>> > OF_function calls to their respective implementation. But in RTEMS
>> > this file maps the OF_functions directly to their FDT implementation.
>> > ---
>> >  cpukit/libfreebsd/freebsd/dev/ofw/ofw_if.h | 62 ++++++++++++++++++++++
>> >  1 file changed, 62 insertions(+)
>> >  create mode 100644 cpukit/libfreebsd/freebsd/dev/ofw/ofw_if.h
>>
>> You may have mentioned this elsewhere, but if this header file needs
>> to be installed it should go under the include/ tree.
>
>
> This header is not intended to be installed.
>
>>
>> >
>> > diff --git a/cpukit/libfreebsd/freebsd/dev/ofw/ofw_if.h b/cpukit/libfreebsd/freebsd/dev/ofw/ofw_if.h
>> > new file mode 100644
>> > index 0000000000..f66c02884e
>> > --- /dev/null
>> > +++ b/cpukit/libfreebsd/freebsd/dev/ofw/ofw_if.h
>> > @@ -0,0 +1,62 @@
>> > +/* SPDX-License-Identifier: BSD-2-Clause
>> > +
>> > + *
>> > + * @file
>> > + *
>> > + * @ingroup LIBFREEBSD
>> These usually aren't all caps. They're not consistent, but LibFreeBSD
>> makes more sense to me.
>
> OK. I'll change this to LibFreeBSD everywhere.
>
>>
>> > + *
>> > + * @brief Declaration of OFW functions.
>> > + */
>> > +
>> > +/*
>> > + * Copyright (C) <2020> Niteesh Babu <niteesh.gs at gmail.com>
>> > + *
>> > + * Redistribution and use in source and binary forms, with or without
>> > + * modification, are permitted provided that the following conditions
>> > + * are met:
>> > + * 1. Redistributions of source code must retain the above copyright
>> > + *    notice, this list of conditions and the following disclaimer.
>> > + * 2. Redistributions in binary form must reproduce the above copyright
>> > + *    notice, this list of conditions and the following disclaimer in the
>> > + *    documentation and/or other materials provided with the distribution.
>> > + *
>> > + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
>> > + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
>> > + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
>> > + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE
>> > + * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
>> > + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
>> > + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
>> > + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
>> > + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
>> > + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
>> > + * POSSIBILITY OF SUCH DAMAGE.
>> > + */
>> > +
>> > +#ifndef _LIBFREEBSD_OFW_IF_H
>> > +#define _LIBFREEBSD_OFW_IF_H
>> > +
>> > +#include <dev/ofw/openfirm.h>
>>
>> This means that header is installed. Please put the headers under the
>> include/dev tree instead.
>
>
> Why does this mean the header is installed?

I meant that openfirm.h is installed. I saw later that you put it in
the include/ directory. However the final outcome remains pending our
discussion about cpukit vs bsps/ and whether there needs to be any
public-facing headers.

> In FreeBSD, these two files are present in the same directory. The openfirm.h
> is what provides the OFW API and ofw_if.h is responsible for calling the correct
> implementation of OFW using KOBJs in case of RTEMS this is hard-wired to the
> FDT implementation in libBSD as well as RTEMS.
> Previously, openfirm.h and ofw_if.h were in the same directory but since I had
> to install openfirm.h I moved it cpukit/include.
> And if this helps, the include is present only to provide a definition for phandle_t.
>
>> > +
>> > +typedef void*  ofw_t;
>> > +
>> > +int OFW_INIT(ofw_t ofw_obj, void *cookie);
>> > +phandle_t OFW_PEER(ofw_t ofw_obj, phandle_t node);
>> > +phandle_t OFW_CHILD(ofw_t ofw_obj, phandle_t node);
>> > +phandle_t OFW_PARENT(ofw_t ofw_obj, phandle_t node);
>> > +phandle_t OFW_INSTANCE_TO_PACKAGE(ofw_t ofw_obj, ihandle_t instance);
>> > +ssize_t OFW_GETPROPLEN(ofw_t ofw_obj, phandle_t package, const char *propname);
>> > +ssize_t OFW_GETPROP(ofw_t ofw_obj, phandle_t package, const char *propname,
>> > +           void *buf, size_t buflen);
>> > +int OFW_NEXTPROP(ofw_t ofw_obj, phandle_t package, const char *prev, char *buf,
>> > +           size_t size);
>> > +int OFW_SETPROP(ofw_t ofw_obj, phandle_t package, const char *propname,
>> > +           const void *buf, size_t len);
>> > +ssize_t OFW_CANON(ofw_t ofw_obj, const char *device, char *buf, size_t len);
>> > +phandle_t OFW_FINDDEVICE(ofw_t ofw_obj, const char *device);
>> > +ssize_t OFW_INSTANCE_TO_PATH(ofw_t ofw_obj, ihandle_t instance, char *buf,
>> > +           size_t len);
>> > +ssize_t OFW_PACKAGE_TO_PATH(ofw_t ofw_obj, phandle_t package, char *buf,
>> > +           size_t len);
>> > +
>>
>> Doxygen?
>
>
> I don't think this is the right place for Doxygen since as mentioned above this
> is not exposed. A better place would be openfirm.h.

We use doxygen for our internal documentation. If a BSP developer is
expected maybe to provide implementations of these functions, then
they should be documented. Maybe, it is sufficient to just document
the interfaces like OF_* from openfirmware.h.

>>
>>
>> > +#endif /* _LIBFREEBSD_OFW_IF_H */
>> > --
>> > 2.17.1
>> >