hyperv/hn: Fix typo in comment
ClosedPublic
Actions

Authored by sepherosa_gmail.com on Feb 5 2016, 6:07 AM.

Details

Reviewers

howard0su_gmail.com
honzhan_microsoft.com
decui_microsoft.com
avos
delphij
royger
adrian

Commits

rS295606: hyperv/hn: Fix typo in comment

Summary

Noticed by: avos

Diff Detail

Lint

Lint Skipped

Unit

Tests Skipped

Event Timeline

sepherosa_gmail.com updated this revision to Diff 13042.Feb 5 2016, 6:07 AM

sepherosa_gmail.com retitled this revision from to hyperv/hn: Fix typo in comment.

sepherosa_gmail.com updated this object.

sepherosa_gmail.com edited the test plan for this revision. (Show Details)

sepherosa_gmail.com added reviewers: adrian, delphij, royger, decui_microsoft.com, honzhan_microsoft.com, howard0su_gmail.com, avos.

honzhan_microsoft.com accepted this revision.Feb 5 2016, 6:25 AM

honzhan_microsoft.com edited edge metadata.

This revision is now accepted and ready to land.Feb 5 2016, 6:25 AM

adrian accepted this revision.Feb 6 2016, 6:19 PM

adrian edited edge metadata.

avos accepted this revision.Feb 13 2016, 9:09 PM

avos edited edge metadata.

Closed by commit rS295606: hyperv/hn: Fix typo in comment (authored by sephe). · Explain WhyFeb 14 2016, 2:29 AM

This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Path

Size

sys/

dev/

hyperv/

netvsc/

hv_netvsc_drv_freebsd.c

2 lines

Diff 13042

View Options

sys/dev/hyperv/netvsc/hv_netvsc_drv_freebsd.c

This file was added.

				/*-
				* Copyright (c) 2022 Takanori Watanabe
				*
				* 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 AUTHOR 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 AUTHOR 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.
				*/

				#include <sys/cdefs.h>
				__FBSDID("$FreeBSD$");

				#include "opt_acpi.h"

				#include <sys/param.h>
				andrewUnsubmitted Done Inline Actions Is `opt_evdev.h` needed? andrew: Is `opt_evdev.h` needed?
				#include <sys/bus.h>
				#include <sys/kernel.h>
				#include <sys/malloc.h>
				#include <sys/module.h>
				#include <sys/rman.h>

				#include <contrib/dev/acpica/include/acpi.h>
				#include <contrib/dev/acpica/include/accommon.h>
				#include <dev/acpica/acpivar.h>

				/* Hooks for the ACPI CA debugging infrastructure */
				#define _COMPONENT ACPI_GED
				ACPI_MODULE_NAME("GED")

				static MALLOC_DEFINE(M_ACPIGED, "acpiged", "ACPI Generic event data");

				struct acpi_ged_events {
				device_t dev;
				hrsUnsubmitted Done Inline Actions I suggest s/events/event/ because this structure is for a single event. hrs: I suggest s/events/event/ because this structure is for a single event.
				struct resource *r;
				int rid;
				void *cookie;
				};

				struct acpi_ged_softc {
				int numevts;
				struct acpi_ged_events *evts;
				};

				static int acpi_ged_probe(device_t dev);
				static int acpi_ged_attach(device_t dev);
				static int acpi_ged_detach(device_t dev);

				static char *ged_ids[] = { "ACPI0013", NULL };

				static device_method_t acpi_ged_methods[] = {
				/* Device interface */
				DEVMETHOD(device_probe, acpi_ged_probe),
				DEVMETHOD(device_attach, acpi_ged_attach),
				DEVMETHOD(device_detach, acpi_ged_detach), DEVMETHOD_END
				};

				hrsUnsubmitted Done Inline Actions DEVMETHOD_END should be on a separate line. hrs: DEVMETHOD_END should be on a separate line.
				static driver_t acpi_ged_driver = {
				"acpi_ged",
				acpi_ged_methods,
				sizeof(struct acpi_ged_softc),
				};

				DRIVER_MODULE(acpi_ged, acpi, acpi_ged_driver, 0, 0);
				MODULE_DEPEND(acpi_ged, acpi, 1, 1, 1);

				static void
				acpi_ged_evt(void *arg)
				{
				struct acpi_ged_events *evt = arg;
				device_t dev = evt->dev;
				ACPI_OBJECT_LIST args;
				ACPI_OBJECT arg1;
				unsigned long irq ;
				#ifdef INTRNG
				struct intr_map_data_acpi *ima;
				ima = rman_get_virtual(evt->r);
				irq = ima->irq;
				#else
				irq = rman_get_start(evt->r);
				#endif

				hrsUnsubmitted Done Inline Actions I am not sure if this matters, but according to the specification[1], a _CRS can have _Exx or _Lxx method, instead of _EVT, for the interrupt-signaled event with an event number <= 255. Also, in the case that there is no _Exx/_Lxx/_EVT method, acpi_ged_evt() is called, and it fails. Registering such event handlers does not make sense. Considering them, it looks better for me to get the ACPI handle for the method, put it into "struct acpi_ged_event" during acpi_ged_attach(), and call it upon an interrupt. If we cannot locate the ACPI handle, we should ignore it with displaying a warning. [1] https://uefi.org/htmlspecs/ACPI_Spec_6_4_html/05_ACPI_Software_Programming_Model/ACPI_Software_Programming_Model.html#crs-object-for-interrupt-signaled-events hrs: I am not sure if this matters, but according to the specification[1], a _CRS can have _Exx or…
				arg1.Type = ACPI_TYPE_INTEGER;
				arg1.Integer.Value = irq;
				args.Count = 1;
				args.Pointer = &arg1;

				AcpiEvaluateObject(acpi_get_handle(dev), "_EVT", &args, NULL);
				}

				static void
				acpi_ged_intr(void *arg)
				{
				AcpiOsExecute(OSL_GPE_HANDLER, acpi_ged_evt, arg);
				}
				static int
				acpi_ged_probe(device_t dev)
				{
				int rv;

				if (acpi_disabled("ged"))
				return (ENXIO);
				rv = ACPI_ID_PROBE(device_get_parent(dev), dev, ged_ids, NULL);
				if (rv > 0)
				return (ENXIO);

				device_set_desc(dev, "Generic Event Device");
				return (rv);
				}

				static int
				acpi_ged_attach(device_t dev)
				{
				struct acpi_ged_softc *sc = device_get_softc(dev);
				struct resource_list *rl;
				struct resource_list_entry *rle;
				int i;
				andrewUnsubmitted Done Inline Actions No need to zero this, the softc will be zero when allocated. andrew: No need to zero this, the softc will be zero when allocated.

				ACPI_FUNCTION_TRACE((char *)(uintptr_t) __func__);

				rl = BUS_GET_RESOURCE_LIST(device_get_parent(dev), dev);
				STAILQ_FOREACH (rle, rl, link) {
				if (rle->type == SYS_RES_IRQ) {
				sc->numevts++;
				}
				}
				sc->evts = mallocarray(sc->numevts, sizeof(*sc->evts), M_ACPIGED,
				hrsUnsubmitted Done Inline Actions I wonder why we need to allocate an IRQ with no RF_SHAREABLE (or without checking whether it can be RF_SHAREABLE). Is there any reason? hrs: I wonder why we need to allocate an IRQ with no RF_SHAREABLE (or without checking whether it…
				M_WAITOK \| M_ZERO);
				for (i = 0; i < sc->numevts; i++) {
				sc->evts[i].dev = dev;
				sc->evts[i].rid = i;
				sc->evts[i].r = bus_alloc_resource_any(dev, SYS_RES_IRQ,
				&sc->evts[i].rid, RF_ACTIVE);
				if (sc->evts[i].r == NULL) {
				device_printf(dev, "Cannot alloc %dth irq\n", i);
				continue;
				}
				if (bus_setup_intr(dev, sc->evts[i].r,
				INTR_TYPE_MISC \| INTR_MPSAFE, NULL, acpi_ged_intr,
				&sc->evts[i], &sc->evts[i].cookie) != 0) {
				device_printf(dev, "Failed to setup intr %d\n", i);
				}
				}

				return_VALUE(0);
				}

				static int
				acpi_ged_detach(device_t dev)
				{
				struct acpi_ged_softc *sc = device_get_softc(dev);
				int i;

				for (i = 0; i < sc->numevts; i++) {
				if (sc->evts[i].cookie) {
				bus_teardown_intr(dev, sc->evts[i].r,
				sc->evts[i].cookie);
				}
				if (sc->evts[i].r) {
				bus_release_resource(dev, SYS_RES_IRQ, sc->evts[i].rid,
				sc->evts[i].r);
				}
				}
				free(sc->evts, M_ACPIGED);

				return 0;
				}
				cyUnsubmitted Not Done Inline Actions Should evt be evts here? cy: Should evt be evts here?

hyperv/hn: Fix typo in commentClosedPublicActions